Clean up e2e test assertion (#264)

* Clean up e2e test assertion

Heavily rely on assert package as well as unit tests

* Iterate over a line for ensuring valid output

* Use subtests for easily spotting the error location

test/e2e/basic_workflow_test.go

@@ -20,6 +20,9 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/knative/client/pkg/util"
+	"gotest.tools/assert"
 )
 
 var (
@@ -46,140 +49,127 @@ func TestBasicWorkflow(t *testing.T) {
 	teardown := Setup(t)
 	defer teardown(t)
 
+	t.Run("returns no service before running tests", func(t *testing.T) {
 		testServiceListEmpty(t, k)
+	})
+
+	t.Run("create hello service and returns no error", func(t *testing.T) {
 		testServiceCreate(t, k, "hello")
+	})
+
+	t.Run("returns valid info about hello service", func(t *testing.T) {
 		testServiceList(t, k, "hello")
 		testServiceDescribe(t, k, "hello")
+	})
+
+	t.Run("update hello service's configuration and returns no error", func(t *testing.T) {
 		testServiceUpdate(t, k, "hello", []string{"--env", "TARGET=kn", "--port", "8888"})
+	})
+
+	t.Run("create another service and returns no error", func(t *testing.T) {
 		testServiceCreate(t, k, "svc2")
+	})
+
+	t.Run("returns a list of revisions associated with hello and svc2 services", func(t *testing.T) {
 		testRevisionListForService(t, k, "hello")
 		testRevisionListForService(t, k, "svc2")
+	})
+
+	t.Run("returns a list of routes associated with hello and svc2 services", func(t *testing.T) {
 		testRouteList(t, k)
 		testRouteListWithArgument(t, k, "hello")
+	})
+
+	t.Run("delete hello and svc2 services and returns no error", func(t *testing.T) {
 		testServiceDelete(t, k, "hello")
 		testServiceDelete(t, k, "svc2")
+	})
+
+	t.Run("returns no service after completing tests", func(t *testing.T) {
 		testServiceListEmpty(t, k)
+	})
 }
 
 // Private test functions
 
 func testServiceListEmpty(t *testing.T, k kn) {
 	out, err := k.RunWithOpts([]string{"service", "list"}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service list' command. Error: %s", err.Error()))
-	}
 
-	if !strings.Contains(out, "No resources found.") {
+	assert.Check(t, util.ContainsAll(out, "No resources found."))
-		t.Fatalf("Expected output 'No resources found.' Instead found:\n%s\n", out)
-	}
 }
 
 func testServiceCreate(t *testing.T, k kn, serviceName string) {
 	out, err := k.RunWithOpts([]string{"service", "create",
 		fmt.Sprintf("%s", serviceName),
 		"--image", KnDefaultTestImage}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service create' command. Error: %s", err.Error()))
-	}
 
-	if !strings.Contains(out, fmt.Sprintf("Service '%s' successfully created in namespace '%s'.", serviceName, k.namespace)) {
+	assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully created in namespace", k.namespace, "OK"))
-		t.Fatalf(fmt.Sprintf("Expected to find: Service '%s' successfully created in namespace '%s'. Instead found:\n%s\n", serviceName, k.namespace, out))
-	}
 }
 
 func testServiceList(t *testing.T, k kn, serviceName string) {
 	out, err := k.RunWithOpts([]string{"service", "list", serviceName}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service list %s' command. Error: %s", serviceName, err.Error()))
-	}
 
 	expectedOutput := fmt.Sprintf("%s", serviceName)
-	if !strings.Contains(out, expectedOutput) {
+	assert.Check(t, util.ContainsAll(out, expectedOutput))
-		t.Fatalf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutput, out)
-	}
 }
 
 func testRevisionListForService(t *testing.T, k kn, serviceName string) {
 	out, err := k.RunWithOpts([]string{"revision", "list", "-s", serviceName}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn revision list -s %s' command. Error: %s", serviceName, err.Error()))
+
-	}
 	outputLines := strings.Split(out, "\n")
-	for _, line := range outputLines[1:] {
+	// Ignore the last line because it is an empty string caused by splitting a line break
-		if len(line) > 1 && !strings.HasPrefix(line, serviceName) {
+	// at the end of the output string
-			t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting line to start with service name: %s\nFound: %s", serviceName, line))
+	for _, line := range outputLines[1 : len(outputLines)-1] {
-		}
+		// The last item is the revision status, which should be ready
+		assert.Check(t, util.ContainsAll(line, " "+serviceName+" ", "True"))
 	}
 }
 
 func testServiceDescribe(t *testing.T, k kn, serviceName string) {
 	out, err := k.RunWithOpts([]string{"service", "describe", serviceName}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service describe' command. Error: %s", err.Error()))
-	}
 
 	expectedOutputHeader := `apiVersion: serving.knative.dev/v1alpha1
 kind: Service
 metadata:`
-	if !strings.Contains(out, expectedOutputHeader) {
-		t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutputHeader, out))
-	}
-
 	expectedOutput := `generation: 1
   name: %s
   namespace: %s`
 	expectedOutput = fmt.Sprintf(expectedOutput, serviceName, k.namespace)
-	if !strings.Contains(out, expectedOutput) {
+	assert.Check(t, util.ContainsAll(out, expectedOutputHeader, expectedOutput))
-		t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutput, out))
-	}
 }
 
 func testServiceUpdate(t *testing.T, k kn, serviceName string, args []string) {
 	out, err := k.RunWithOpts(append([]string{"service", "update", serviceName}, args...), runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service update' command. Error: %s", err.Error()))
+
-	}
 	expectedOutput := fmt.Sprintf("Service '%s' updated", serviceName)
-	if !strings.Contains(out, expectedOutput) {
+	assert.Check(t, util.ContainsAll(out, expectedOutput))
-		t.Fatalf(fmt.Sprintf("Expected output incorrect, expecting to include:\n%s\nFound:\n%s\n", expectedOutput, out))
-	}
 }
 
 func testRouteList(t *testing.T, k kn) {
 	out, err := k.RunWithOpts([]string{"route", "list"}, runOpts{})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Errorf(fmt.Sprintf("Error executing 'kn route list' command. Error: %s", err.Error()))
+
-	}
 	expectedHeaders := []string{"NAME", "URL", "AGE", "CONDITIONS", "TRAFFIC"}
-	for _, header := range expectedHeaders {
+	assert.Check(t, util.ContainsAll(out, expectedHeaders...))
-		if !strings.Contains(out, header) {
-			t.Errorf("Expected to include header %s in 'kn route list' output. Actual output:\n%s\n", header, out)
-		}
-	}
 }
 
 func testRouteListWithArgument(t *testing.T, k kn, routeName string) {
 	out, err := k.RunWithOpts([]string{"route", "list", routeName}, runOpts{})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Errorf("Error executing 'kn route list %s' command. Error: %s", routeName, err.Error())
+
-	}
+	expectedOutput := fmt.Sprintf("100%% -> %s", routeName)
-	expectedOutput := routeName
+	assert.Check(t, util.ContainsAll(out, routeName, expectedOutput))
-	if !strings.Contains(out, expectedOutput) {
-		t.Errorf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutput, out)
-	}
-	expectedOutput = fmt.Sprintf("100%% -> %s", routeName)
-	if !strings.Contains(out, expectedOutput) {
-		t.Errorf("Expected output incorrect, expecting to include:\n%s\n Instead found:\n%s\n", expectedOutput, out)
-	}
 }
 
 func testServiceDelete(t *testing.T, k kn, serviceName string) {
 	out, err := k.RunWithOpts([]string{"service", "delete", serviceName}, runOpts{NoNamespace: false})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Fatalf(fmt.Sprintf("Error executing 'kn service delete' command. Error: %s", err.Error()))
-	}
 
-	if !strings.Contains(out, fmt.Sprintf("Service '%s' successfully deleted in namespace '%s'.", serviceName, k.namespace)) {
+	assert.Check(t, util.ContainsAll(out, "Service", serviceName, "successfully deleted in namespace", k.namespace))
-		t.Fatalf(fmt.Sprintf("Expected to find: Service '%s' successfully deleted in namespace '%s'. Instead found:\n%s\n", serviceName, k.namespace, out))
-	}
 }

test/e2e/revision_workflow_test.go

@@ -28,22 +28,28 @@ func TestRevisionWorkflow(t *testing.T) {
 	teardown := Setup(t)
 	defer teardown(t)
 
+	t.Run("create hello service and returns no error", func(t *testing.T) {
 		testServiceCreate(t, k, "hello")
+	})
+
+	t.Run("delete latest revision from hello service and returns no error", func(t *testing.T) {
 		testDeleteRevision(t, k, "hello")
+	})
+
+	t.Run("delete hello service and returns no error", func(t *testing.T) {
 		testServiceDelete(t, k, "hello")
+	})
 }
 
 func testDeleteRevision(t *testing.T, k kn, serviceName string) {
 	revName, err := k.RunWithOpts([]string{"revision", "list", "-o=jsonpath={.items[0].metadata.name}"}, runOpts{})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Errorf("Error executing 'revision list -o' command. Error: %s", err.Error())
-	}
 	if strings.Contains(revName, "No resources found.") {
 		t.Errorf("Could not find revision name.")
 	}
+
 	out, err := k.RunWithOpts([]string{"revision", "delete", revName}, runOpts{})
-	if err != nil {
+	assert.NilError(t, err)
-		t.Errorf("Error executing 'revision delete %s' command. Error: %s", revName, err.Error())
+
-	}
 	assert.Check(t, util.ContainsAll(out, "Revision", revName, "deleted", "namespace", k.namespace))
 }

test/e2e/service_options_test.go

@@ -28,12 +28,26 @@ func TestServiceOptions(t *testing.T) {
 	teardown := Setup(t)
 	defer teardown(t)
 
+	t.Run("create hello service with concurrency options and returns no error", func(t *testing.T) {
 		testServiceCreateWithOptions(t, k, "hello", []string{"--concurrency-limit", "250", "--concurrency-target", "300"})
+	})
+
+	t.Run("returns valid concurrency options for hello service", func(t *testing.T) {
 		testServiceDescribeConcurrencyLimit(t, k, "hello", "250")
 		testServiceDescribeConcurrencyTarget(t, k, "hello", "300")
+	})
+
+	t.Run("update concurrency limit for hello service and returns no error", func(t *testing.T) {
 		testServiceUpdate(t, k, "hello", []string{"--concurrency-limit", "300"})
+	})
+
+	t.Run("returns correct concurrency limit for hello service", func(t *testing.T) {
 		testServiceDescribeConcurrencyLimit(t, k, "hello", "300")
+	})
+
+	t.Run("delete hello service and returns no error", func(t *testing.T) {
 		testServiceDelete(t, k, "hello")
+	})
 }
 
 func testServiceCreateWithOptions(t *testing.T, k kn, serviceName string, options []string) {

test/e2e/version_test.go

@@ -17,8 +17,10 @@
 package e2e
 
 import (
-	"strings"
 	"testing"
+
+	"github.com/knative/client/pkg/util"
+	"gotest.tools/assert"
 )
 
 func TestVersion(t *testing.T) {
@@ -27,7 +29,5 @@ func TestVersion(t *testing.T) {
 
 	out, _ := kn.RunWithOpts([]string{"version"}, runOpts{NoNamespace: true})
 
-	if !strings.Contains(out, "Version") {
+	assert.Check(t, util.ContainsAll(out, "Version"))
-		t.Fatalf("Expected to find client version")
-	}
 }