wsclient: Add test to getClientServer() in client tests, make comments as complete sentences

Author: Richa Gangwar

ecs-agent/wsclient/client_test.go

@@ -46,7 +46,7 @@ const dockerEndpoint = "/var/run/docker.sock"
 
 // Close closes the underlying connection. Implement Close() in this file
 // as ClientServerImpl doesn't implement it. This is needed by the
-// TestSetReadDeadline* tests
+// TestSetReadDeadline* tests.
 func (cs *ClientServerImpl) Close() error {
 	return cs.Disconnect()
 }
@@ -57,15 +57,15 @@ func TestClientProxy(t *testing.T) {
 	defer os.Unsetenv("HTTP_PROXY")
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer("http://www.amazon.com", types)
+	cs := getTestClientServer("http://www.amazon.com", types)
 	err := cs.Connect()
 	assert.Error(t, err)
 	assert.True(t, strings.Contains(err.Error(), proxy_url), "proxy not found: %s", err.Error())
 }
 
 // TestConcurrentWritesDontPanic will force a panic in the websocket library if
-// the implemented methods don't handle concurrency correctly
-// See https://godoc.org/github.com/gorilla/websocket#hdr-Concurrency
+// the implemented methods don't handle concurrency correctly.
+// See https://godoc.org/github.com/gorilla/websocket#hdr-Concurrency.
 func TestConcurrentWritesDontPanic(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -86,7 +86,7 @@ func TestConcurrentWritesDontPanic(t *testing.T) {
 	req := ecsacs.AckRequest{Cluster: aws.String("test"), ContainerInstance: aws.String("test"), MessageId: aws.String("test")}
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	require.NoError(t, cs.Connect())
 
 	executeTenRequests := func() {
@@ -96,16 +96,15 @@ func TestConcurrentWritesDontPanic(t *testing.T) {
 	}
 
 	// Make requests from two separate routines to try and force a
-	// concurrent write
+	// concurrent write.
 	go executeTenRequests()
 	go executeTenRequests()
 
 	t.Log("Waiting for all 20 requests to succeed")
 	waitForRequests.Wait()
 }
 
-func getClientServer(url string, msgType []interface{}) *ClientServerImpl {
-
+func getTestClientServer(url string, msgType []interface{}) *ClientServerImpl {
 	testCreds := credentials.NewStaticCredentials("test-id", "test-secret", "test-token")
 
 	return &ClientServerImpl{
@@ -124,7 +123,7 @@ func getClientServer(url string, msgType []interface{}) *ClientServerImpl {
 }
 
 // TestProxyVariableCustomValue ensures that a user is able to override the
-// proxy variable by setting an environment variable
+// proxy variable by setting an environment variable.
 func TestProxyVariableCustomValue(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -136,13 +135,13 @@ func TestProxyVariableCustomValue(t *testing.T) {
 	testString := "Custom no proxy string"
 	os.Setenv("NO_PROXY", testString)
 	types := []interface{}{ecsacs.AckRequest{}}
-	require.NoError(t, getClientServer(mockServer.URL, types).Connect())
+	require.NoError(t, getTestClientServer(mockServer.URL, types).Connect())
 
 	assert.Equal(t, os.Getenv("NO_PROXY"), testString, "NO_PROXY should match user-supplied variable")
 }
 
 // TestProxyVariableDefaultValue verifies that NO_PROXY gets overridden if it
-// isn't already set
+// isn't already set.
 func TestProxyVariableDefaultValue(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -153,15 +152,15 @@ func TestProxyVariableDefaultValue(t *testing.T) {
 
 	os.Unsetenv("NO_PROXY")
 	types := []interface{}{ecsacs.AckRequest{}}
-	getClientServer(mockServer.URL, types).Connect()
+	getTestClientServer(mockServer.URL, types).Connect()
 
 	expectedEnvVar := "169.254.169.254,169.254.170.2," + dockerEndpoint
 
 	assert.Equal(t, os.Getenv("NO_PROXY"), expectedEnvVar, "Variable NO_PROXY expected to be overwritten when no default value supplied")
 }
 
 // TestHandleMessagePermissibleCloseCode ensures that permissible close codes
-// are wrapped in io.EOF
+// are wrapped in io.EOF.
 func TestHandleMessagePermissibleCloseCode(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -172,7 +171,7 @@ func TestHandleMessagePermissibleCloseCode(t *testing.T) {
 	mockServer.StartTLS()
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	require.NoError(t, cs.Connect())
 	assert.True(t, cs.IsReady(), "expected websocket connection to be ready")
 
@@ -185,7 +184,7 @@ func TestHandleMessagePermissibleCloseCode(t *testing.T) {
 }
 
 // TestHandleMessageUnexpectedCloseCode checks that unexpected close codes will
-// be returned as is (not wrapped in io.EOF)
+// be returned as is (not wrapped in io.EOF).
 func TestHandleMessageUnexpectedCloseCode(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -195,7 +194,7 @@ func TestHandleMessageUnexpectedCloseCode(t *testing.T) {
 	mockServer.StartTLS()
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	require.NoError(t, cs.Connect())
 	assert.True(t, cs.IsReady(), "expected websocket connection to be ready")
 
@@ -209,7 +208,7 @@ func TestHandleMessageUnexpectedCloseCode(t *testing.T) {
 }
 
 // TestHandlNonHTTPSEndpoint verifies that the wsclient can handle communication over
-// an HTTP (so WS) connection
+// an HTTP (so WS) connection.
 func TestHandleNonHTTPSEndpoint(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -219,7 +218,7 @@ func TestHandleNonHTTPSEndpoint(t *testing.T) {
 	defer mockServer.Close()
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	require.NoError(t, cs.Connect())
 	assert.True(t, cs.IsReady(), "expected websocket connection to be ready")
 
@@ -231,7 +230,7 @@ func TestHandleNonHTTPSEndpoint(t *testing.T) {
 }
 
 // TestHandleIncorrectHttpScheme checks that an incorrect URL scheme results in
-// an error
+// an error.
 func TestHandleIncorrectURLScheme(t *testing.T) {
 	closeWS := make(chan []byte)
 	defer close(closeWS)
@@ -244,16 +243,16 @@ func TestHandleIncorrectURLScheme(t *testing.T) {
 	mockServerURL.Scheme = "notaparticularlyrealscheme"
 
 	types := []interface{}{ecsacs.AckRequest{}}
-	cs := getClientServer(mockServerURL.String(), types)
+	cs := getTestClientServer(mockServerURL.String(), types)
 	err := cs.Connect()
 
 	assert.Error(t, err, "Expected error for incorrect URL scheme")
 }
 
 // TestWebsocketScheme checks that websocketScheme handles valid and invalid mappings
-// correctly
+// correctly.
 func TestWebsocketScheme(t *testing.T) {
-	// test valid schemes
+	// Test valid schemes.
 	validMappings := map[string]string{
 		"http":  "ws",
 		"https": "wss",
@@ -266,7 +265,7 @@ func TestWebsocketScheme(t *testing.T) {
 		assert.Equal(t, actualOutput, expectedOutput, "Valid http schemes should map to a websocket scheme")
 	}
 
-	// test an invalid mapping
+	// Test an invalid mapping.
 	_, err := websocketScheme("highly-likely-to-be-junk")
 	assert.Error(t, err, "Expected error for invalid http scheme")
 }
@@ -322,7 +321,7 @@ func TestAddRequestPayloadHandler(t *testing.T) {
 
 	types := []interface{}{ecsacs.PayloadMessage{}}
 	messageError := make(chan error)
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	cs.conn = conn
 
 	defer cs.Close()
@@ -365,7 +364,7 @@ func TestMakeUnrecognizedRequest(t *testing.T) {
 	mockServer.StartTLS()
 
 	types := []interface{}{ecsacs.PayloadMessage{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	cs.conn = conn
 
 	defer cs.Close()
@@ -389,7 +388,7 @@ func TestWriteCloseMessage(t *testing.T) {
 	mockServer.StartTLS()
 
 	types := []interface{}{ecsacs.PayloadMessage{}}
-	cs := getClientServer(mockServer.URL, types)
+	cs := getTestClientServer(mockServer.URL, types)
 	cs.Connect()
 
 	defer cs.Close()