various updates to address reviewers feedback

CHANGELOG.adoc

@@ -20,8 +20,8 @@
 |===
 
 | 🐣
-| Refactor `e2e` common code into `lib\test\integration`
-| https://github.com/knative/client/pull/764[#764]
+| Refactor `e2e` common code into `lib/test/integration`
+| https://github.com/knative/client/pull/765[#765]
 
 ## v0.13.1 (2020-03-25)
 

lib/test/integration/cli.go

@@ -28,6 +28,7 @@ const (
 	seperatorLight = "╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍╍"
 )
 
+// Kn type
 type Kn struct {
 	namespace string
 }
@@ -37,7 +38,7 @@ func NewKn() Kn {
 	return Kn{}
 }
 
-// Run the 'kn' CLI with args and opts
+// Run the 'kn' CLI with args
 func (k Kn) Run(args ...string) KnRunResult {
 	return RunKn(k.namespace, args)
 }
@@ -47,24 +48,24 @@ func (k Kn) Namespace() string {
 	return k.namespace
 }
 
-// Helper methods for calling out to the test cluster
+// Kubectl type
 type Kubectl struct {
 	namespace string
 }
 
-// New Kn object
+// New Kubectl object
 func NewKubectl(namespace string) Kubectl {
 	return Kubectl{
 		namespace: namespace,
 	}
 }
 
-// Run the 'kubectl' CLI with args and opts
+// Run the 'kubectl' CLI with args
 func (k Kubectl) Run(args ...string) (string, error) {
 	return RunKubectl(k.namespace, args...)
 }
 
-// Namespace that this Kn instance uses
+// Namespace that this Kubectl instance uses
 func (k Kubectl) Namespace() string {
 	return k.namespace
 }

lib/test/integration/integration.go

@@ -38,24 +38,14 @@ var serviceMutex sync.Mutex
 var serviceCount int
 var namespaceCount int
 
-// IntegrationTest struct
-type Test struct {
+// KnTest type
+type KnTest struct {
 	namespace string
 	kn        Kn
 }
 
-// Teardown clean up
-func (test *Test) Teardown() error {
-	return DeleteNamespace(test.namespace)
-}
-
-// Teardown clean up
-func (test *Test) Kn() Kn {
-	return test.kn
-}
-
 // NewIntegrationTest creates a new ItegrationTest object
-func NewIntegrationTest() (*Test, error) {
+func NewKnTest() (*KnTest, error) {
 	ns := NextNamespace()
 
 	err := CreateNamespace(ns)
@@ -67,12 +57,24 @@ func NewIntegrationTest() (*Test, error) {
 		return nil, err
 	}
 
-	return &Test{
+	return &KnTest{
 		namespace: ns,
 		kn:        Kn{ns},
 	}, nil
 }
 
+// Teardown clean up
+func (test *KnTest) Teardown() error {
+	return DeleteNamespace(test.namespace)
+}
+
+// Teardown clean up
+func (test *KnTest) Kn() Kn {
+	return test.kn
+}
+
+// Public functions
+
 // NextNamespace return the next unique namespace
 func NextNamespace() string {
 	ns := os.Getenv("KN_E2E_NAMESPACE")

lib/test/integration/result_collector.go

@@ -1,4 +1,4 @@
-// Copyright 2019 The Knative Authors
+// Copyright 2020 The Knative Authors
 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -22,7 +22,7 @@ import (
 	"testing"
 )
 
-// Result of a "kn" call
+// KnRunResult holds command and result artifacts of a "kn" call
 type KnRunResult struct {
 	// Command line called
 	CmdLine string

test/e2e/basic_workflow_test.go

@@ -29,7 +29,7 @@ import (
 
 func TestBasicWorkflow(t *testing.T) {
 	t.Parallel()
-	it, err := integration.NewIntegrationTest()
+	it, err := integration.NewKnTest()
 	assert.NilError(t, err)
 	defer func() {
 		assert.NilError(t, it.Teardown())
@@ -85,47 +85,47 @@ func TestWrongCommand(t *testing.T) {
 
 // ==========================================================================
 
-func serviceListEmpty(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector) {
+func serviceListEmpty(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector) {
 	out := it.Kn().Run("service", "list")
 	r.AssertNoError(out)
 	assert.Check(t, util.ContainsAll(out.Stdout, "No services found."))
 }
 
-func serviceCreate(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func serviceCreate(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	out := it.Kn().Run("service", "create", serviceName, "--image", integration.KnDefaultTestImage)
 	r.AssertNoError(out)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "service", serviceName, "creating", "namespace", it.Kn().Namespace(), "ready"))
 }
 
-func serviceList(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func serviceList(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	out := it.Kn().Run("service", "list", serviceName)
 	r.AssertNoError(out)
 	assert.Check(t, util.ContainsAll(out.Stdout, serviceName))
 }
 
-func serviceDescribe(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func serviceDescribe(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	out := it.Kn().Run("service", "describe", serviceName)
 	r.AssertNoError(out)
 	assert.Assert(t, util.ContainsAll(out.Stdout, serviceName, it.Kn().Namespace(), integration.KnDefaultTestImage))
 	assert.Assert(t, util.ContainsAll(out.Stdout, "Conditions", "ConfigurationsReady", "Ready", "RoutesReady"))
 	assert.Assert(t, util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Age", "Revisions"))
 }
 
-func serviceUpdate(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string, args ...string) {
+func serviceUpdate(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string, args ...string) {
 	fullArgs := append([]string{}, "service", "update", serviceName)
 	fullArgs = append(fullArgs, args...)
 	out := it.Kn().Run(fullArgs...)
 	r.AssertNoError(out)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "updating", "service", serviceName, "ready"))
 }
 
-func serviceDelete(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func serviceDelete(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	out := it.Kn().Run("service", "delete", serviceName)
 	r.AssertNoError(out)
 	assert.Check(t, util.ContainsAll(out.Stdout, "Service", serviceName, "successfully deleted in namespace", it.Kn().Namespace()))
 }
 
-func revisionListForService(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func revisionListForService(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	out := it.Kn().Run("revision", "list", "-s", serviceName)
 	r.AssertNoError(out)
 	outputLines := strings.Split(out.Stdout, "\n")
@@ -137,7 +137,7 @@ func revisionListForService(t *testing.T, it *integration.Test, r *integration.K
 	}
 }
 
-func revisionDescribe(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) {
+func revisionDescribe(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) {
 	revName := findRevision(t, it, r, serviceName)
 
 	out := it.Kn().Run("revision", "describe", revName)

test/e2e/ping_test.go

@@ -28,7 +28,7 @@ import (
 
 func TestSourcePing(t *testing.T) {
 	t.Parallel()
-	it, err := integration.NewIntegrationTest()
+	it, err := integration.NewKnTest()
 	assert.NilError(t, err)
 	defer func() {
 		assert.NilError(t, it.Teardown())
@@ -65,49 +65,49 @@ func TestSourcePing(t *testing.T) {
 	verifyPingSourceDescribe(t, it, r, "testpingsource3", "*/1 * * * *", mymsg, "testsvc1")
 }
 
-func pingSourceCreate(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
+func pingSourceCreate(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
 	out := it.Kn().Run("source", "ping", "create", sourceName,
 		"--schedule", schedule, "--data", data, "--sink", sink)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "ping", "source", sourceName, "created", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 }
 
-func pingSourceDelete(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string) {
+func pingSourceDelete(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string) {
 	out := it.Kn().Run("source", "ping", "delete", sourceName)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "ping", "source", sourceName, "deleted", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 
 }
 
-func pingSourceCreateMissingSink(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
+func pingSourceCreateMissingSink(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
 	out := it.Kn().Run("source", "ping", "create", sourceName,
 		"--schedule", schedule, "--data", data, "--sink", sink)
 	assert.Check(t, util.ContainsAll(out.Stderr, "services.serving.knative.dev", "not found"))
 	r.AssertError(out)
 }
 
-func pingSourceUpdateSink(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, sink string) {
+func pingSourceUpdateSink(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, sink string) {
 	out := it.Kn().Run("source", "ping", "update", sourceName, "--sink", sink)
 	assert.Check(t, util.ContainsAll(out.Stdout, sourceName, "updated", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 }
 
-func pingSourceCreateWithResources(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string, sa string, requestcpu string, requestmm string, limitcpu string, limitmm string) {
+func pingSourceCreateWithResources(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string, sa string, requestcpu string, requestmm string, limitcpu string, limitmm string) {
 	out := it.Kn().Run("source", "ping", "create", sourceName,
 		"--schedule", schedule, "--data", data, "--sink", sink, "--service-account", sa,
 		"--requests-cpu", requestcpu, "--requests-memory", requestmm, "--limits-cpu", limitcpu, "--limits-memory", limitmm)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, "ping", "source", sourceName, "created", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 }
 
-func pingSourceUpdateResources(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, requestcpu string, requestmm string, limitcpu string, limitmm string) {
+func pingSourceUpdateResources(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, requestcpu string, requestmm string, limitcpu string, limitmm string) {
 	out := it.Kn().Run("source", "ping", "update", sourceName,
 		"--requests-cpu", requestcpu, "--requests-memory", requestmm, "--limits-cpu", limitcpu, "--limits-memory", limitmm)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, sourceName, "updated", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 }
 
-func verifyPingSourceDescribe(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
+func verifyPingSourceDescribe(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, sourceName string, schedule string, data string, sink string) {
 	out := it.Kn().Run("source", "ping", "describe", sourceName)
 	assert.Check(t, util.ContainsAllIgnoreCase(out.Stdout, sourceName, schedule, data, sink))
 	r.AssertNoError(out)

test/e2e/revision_test.go

@@ -31,7 +31,7 @@ import (
 
 func TestRevision(t *testing.T) {
 	t.Parallel()
-	it, err := integration.NewIntegrationTest()
+	it, err := integration.NewKnTest()
 	assert.NilError(t, err)
 	defer func() {
 		assert.NilError(t, it.Teardown())
@@ -70,7 +70,7 @@ func TestRevision(t *testing.T) {
 	serviceDelete(t, it, r, "hello")
 }
 
-func revisionListWithService(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceNames ...string) {
+func revisionListWithService(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceNames ...string) {
 	for _, svcName := range serviceNames {
 		confGen := findConfigurationGeneration(t, it, r, svcName)
 		out := it.Kn().Run("revision", "list", "-s", svcName)
@@ -90,13 +90,13 @@ func revisionListWithService(t *testing.T, it *integration.Test, r *integration.
 	}
 }
 
-func revisionDelete(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, revName string) {
+func revisionDelete(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, revName string) {
 	out := it.Kn().Run("revision", "delete", revName)
 	assert.Check(t, util.ContainsAll(out.Stdout, "Revision", revName, "deleted", "namespace", it.Kn().Namespace()))
 	r.AssertNoError(out)
 }
 
-func revisionMultipleDelete(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, existRevision1, existRevision2, nonexistRevision string) {
+func revisionMultipleDelete(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, existRevision1, existRevision2, nonexistRevision string) {
 	out := it.Kn().Run("revision", "list")
 	r.AssertNoError(out)
 	assert.Check(t, strings.Contains(out.Stdout, existRevision1), "Required revision1 does not exist")
@@ -110,14 +110,14 @@ func revisionMultipleDelete(t *testing.T, it *integration.Test, r *integration.K
 	assert.Check(t, util.ContainsAll(out.Stdout, "revisions.serving.knative.dev", nonexistRevision, "not found"), "Failed to get 'not found' error")
 }
 
-func revisionDescribeWithPrintFlags(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, revName string) {
+func revisionDescribeWithPrintFlags(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, revName string) {
 	out := it.Kn().Run("revision", "describe", revName, "-o=name")
 	r.AssertNoError(out)
 	expectedName := fmt.Sprintf("revision.serving.knative.dev/%s", revName)
 	assert.Equal(t, strings.TrimSpace(out.Stdout), expectedName)
 }
 
-func findRevision(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) string {
+func findRevision(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) string {
 	out := it.Kn().Run("revision", "list", "-s", serviceName, "-o=jsonpath={.items[0].metadata.name}")
 	r.AssertNoError(out)
 	if strings.Contains(out.Stdout, "No resources") {
@@ -126,7 +126,7 @@ func findRevision(t *testing.T, it *integration.Test, r *integration.KnRunResult
 	return out.Stdout
 }
 
-func findRevisionByGeneration(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string, generation int) string {
+func findRevisionByGeneration(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string, generation int) string {
 	maxGen := findConfigurationGeneration(t, it, r, serviceName)
 	out := it.Kn().Run("revision", "list", "-s", serviceName,
 		fmt.Sprintf("-o=jsonpath={.items[%d].metadata.name}", maxGen-generation))
@@ -137,7 +137,7 @@ func findRevisionByGeneration(t *testing.T, it *integration.Test, r *integration
 	return out.Stdout
 }
 
-func findConfigurationGeneration(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, serviceName string) int {
+func findConfigurationGeneration(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, serviceName string) int {
 	out := it.Kn().Run("revision", "list", "-s", serviceName, "-o=jsonpath={.items[0].metadata.labels.serving\\.knative\\.dev/configurationGeneration}")
 	r.AssertNoError(out)
 	if out.Stdout == "" {

test/e2e/route_test.go

@@ -30,7 +30,7 @@ import (
 
 func TestRoute(t *testing.T) {
 	t.Parallel()
-	it, err := integration.NewIntegrationTest()
+	it, err := integration.NewKnTest()
 	assert.NilError(t, err)
 	defer func() {
 		assert.NilError(t, it.Teardown())
@@ -61,38 +61,38 @@ func TestRoute(t *testing.T) {
 	serviceDelete(t, it, r, "hello")
 }
 
-func routeList(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector) {
+func routeList(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector) {
 	out := it.Kn().Run("route", "list")
 
 	expectedHeaders := []string{"NAME", "URL", "READY"}
 	assert.Check(t, util.ContainsAll(out.Stdout, expectedHeaders...))
 	r.AssertNoError(out)
 }
 
-func routeListWithArgument(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, routeName string) {
+func routeListWithArgument(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, routeName string) {
 	out := it.Kn().Run("route", "list", routeName)
 
 	assert.Check(t, util.ContainsAll(out.Stdout, routeName))
 	r.AssertNoError(out)
 }
 
-func routeDescribe(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, routeName string) {
+func routeDescribe(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, routeName string) {
 	out := it.Kn().Run("route", "describe", routeName)
 
 	assert.Check(t, util.ContainsAll(out.Stdout,
 		routeName, it.Kn().Namespace(), "URL", "Service", "Traffic", "Targets", "Conditions"))
 	r.AssertNoError(out)
 }
 
-func routeDescribeWithPrintFlags(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, routeName string) {
+func routeDescribeWithPrintFlags(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, routeName string) {
 	out := it.Kn().Run("route", "describe", routeName, "-o=name")
 
 	expectedName := fmt.Sprintf("route.serving.knative.dev/%s", routeName)
 	assert.Equal(t, strings.TrimSpace(out.Stdout), expectedName)
 	r.AssertNoError(out)
 }
 
-func routeListWithPrintFlags(t *testing.T, it *integration.Test, r *integration.KnRunResultCollector, names ...string) {
+func routeListWithPrintFlags(t *testing.T, it *integration.KnTest, r *integration.KnRunResultCollector, names ...string) {
 	out := it.Kn().Run("route", "list", "-o=jsonpath={.items[*].metadata.name}")
 	assert.Check(t, util.ContainsAll(out.Stdout, names...))
 	r.AssertNoError(out)