Repository: knative/client (go)

Issue:
Add E2E test for "kn source binding" commands
<!-- If you need to report a security issue with Knative, send an email to knative-security@googlegroups.com. -->
## In what area(s)?

E2E Testing

<!-- Remove the '> ' to select -->

<!--
Classifications:
/kind good-first-issue
> /kind feature
> /kind proposal
> /kind doc
> /kind cleanup
-->

## Describe the feature:

We need some E2E tests for the CRUD commands of `kn source binding`

Pull request:
Add E2E test for "kn source binding" commands
Fixes #631

## Proposed Changes
* Add E2E test for "kn source binding" commands.
* Fix typo.

Release Note:
- 🎁 Add new feature
Add E2E test for "kn source binding" commands.



Patch:
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index 198f6510..65b842de 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -22,7 +22,11 @@
 
 | 🐛
 | Show envFrom when running describe service or revision
-| https://github.com/knative/client/pull/630
+| https://github.com/knative/client/pull/630[#630]
+
+| 🎁
+| Add E2E test for `kn source binding` commands
+| https://github.com/knative/client/pull/634[#634]
 
 | 🎁
 | Add `--cmd` and `--arg` for customization of container entrypoint
diff --git a/docs/cmd/kn_source_binding_create.md b/docs/cmd/kn_source_binding_create.md
index d4c73913..6d8552c1 100644
--- a/docs/cmd/kn_source_binding_create.md
+++ b/docs/cmd/kn_source_binding_create.md
@@ -15,7 +15,7 @@ kn source binding create NAME --subject SCHEDULE --sink SINK --ce-override KEY=V
 ```
 
   # Create a sink binding which connects a deployment 'myapp' with a Knative service 'mysvc'
-  kn source binding create my-binding --subject Deployemnt:apps/v1:myapp --sink svc:mysvc
+  kn source binding create my-binding --subject Deployment:apps/v1:myapp --sink svc:mysvc
 ```
 
 ### Options
diff --git a/pkg/kn/commands/source/binding/create.go b/pkg/kn/commands/source/binding/create.go
index b8a92397..cde4fada 100644
--- a/pkg/kn/commands/source/binding/create.go
+++ b/pkg/kn/commands/source/binding/create.go
@@ -35,7 +35,7 @@ func NewBindingCreateCommand(p *commands.KnParams) *cobra.Command {
 		Short: "Create a sink binding.",
 		Example: `
   # Create a sink binding which connects a deployment 'myapp' with a Knative service 'mysvc'
-  kn source binding create my-binding --subject Deployemnt:apps/v1:myapp --sink svc:mysvc`,
+  kn source binding create my-binding --subject Deployment:apps/v1:myapp --sink svc:mysvc`,
 
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
 			if len(args) != 1 {
diff --git a/pkg/wait/wait_for_ready.go b/pkg/wait/wait_for_ready.go
index 957fa0f4..d5a5c563 100644
--- a/pkg/wait/wait_for_ready.go
+++ b/pkg/wait/wait_for_ready.go
@@ -25,6 +25,10 @@ import (
 	"knative.dev/pkg/apis"
 )
 
+// Window for how long a ReadyCondition == false has to stay
+// for being considered as an error
+var ErrorWindow = 2 * time.Second
+
 // Callbacks and configuration used while waiting
 type waitForReadyConfig struct {
 	watchMaker          WatchMaker
@@ -87,7 +91,7 @@ func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, msgCallbac
 	floatingTimeout := timeout
 	for {
 		start := time.Now()
-		retry, timeoutReached, err := w.waitForReadyCondition(start, name, floatingTimeout, msgCallback)
+		retry, timeoutReached, err := w.waitForReadyCondition(start, name, floatingTimeout, ErrorWindow, msgCallback)
 		if err != nil {
 			return err, time.Since(start)
 		}
@@ -104,18 +108,31 @@ func (w *waitForReadyConfig) Wait(name string, timeout time.Duration, msgCallbac
 	}
 }
 
-func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string, timeout time.Duration, msgCallback MessageCallback) (retry bool, timeoutReached bool, err error) {
+func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string, timeout time.Duration, errorWindow time.Duration, msgCallback MessageCallback) (retry bool, timeoutReached bool, err error) {
 
 	watcher, err := w.watchMaker(name, timeout)
 	if err != nil {
 		return false, false, err
 	}
-
 	defer watcher.Stop()
+
+	// channel used to transport the error
+	errChan := make(chan error)
+	var errorTimer *time.Timer
+	// Stop error timer if it has been started because of
+	// a ConditionReady has been set to false
+	defer (func() {
+		if errorTimer != nil {
+			errorTimer.Stop()
+		}
+	})()
+
 	for {
 		select {
 		case <-time.After(timeout):
 			return false, true, nil
+		case err = <-errChan:
+			return false, false, err
 		case event, ok := <-watcher.ResultChan():
 			if !ok || event.Object == nil {
 				return true, false, nil
@@ -140,7 +157,15 @@ func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string,
 					case corev1.ConditionTrue:
 						return false, false, nil
 					case corev1.ConditionFalse:
-						return false, false, fmt.Errorf("%s: %s", cond.Reason, cond.Message)
+						// Fire up a timer waiting for the error window duration to still allow to reconcile
+						// to a true condition even after the condition went to false. If this is not the case within
+						// this window, then an error is returned.
+						// If there is already a timer running, we just log.
+						if errorTimer != nil {
+							errorTimer = time.AfterFunc(errorWindow, func() {
+								errChan <- fmt.Errorf("%s: %s", cond.Reason, cond.Message)
+							})
+						}
 					}
 					if cond.Message != "" {
 						msgCallback(time.Since(start), cond.Message)


Test patch:
diff --git a/pkg/wait/wait_for_ready_test.go b/pkg/wait/wait_for_ready_test.go
index fb20f43d..ad80ad19 100644
--- a/pkg/wait/wait_for_ready_test.go
+++ b/pkg/wait/wait_for_ready_test.go
@@ -79,6 +79,7 @@ func prepareTestCases(name string) []waitForReadyTestCase {
 		tc(peError, name, true),
 		tc(peWrongGeneration, name, true),
 		tc(peTimeout, name, true),
+		tc(peReadyFalseWithinErrorWindow, name, false),
 	}
 }
 
@@ -131,3 +132,11 @@ func peWrongGeneration(name string) ([]watch.Event, int) {
 		{watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionTrue, corev1.ConditionTrue, "", "", 1, 2)},
 	}, len(messages)
 }
+
+func peReadyFalseWithinErrorWindow(name string) ([]watch.Event, int) {
+	messages := pMessages(1)
+	return []watch.Event{
+		{watch.Added, CreateTestServiceWithConditions(name, corev1.ConditionFalse, corev1.ConditionFalse, "Route not ready", messages[0])},
+		{watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionTrue, corev1.ConditionTrue, "Route ready", "")},
+	}, len(messages)
+}
diff --git a/test/e2e/source_binding_test.go b/test/e2e/source_binding_test.go
new file mode 100644
index 00000000..3f33ab96
--- /dev/null
+++ b/test/e2e/source_binding_test.go
@@ -0,0 +1,73 @@
+// 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.
+// You may obtain a copy of the License at
+
+//     http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or im
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// +build e2e
+// +build !serving
+
+package e2e
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	"knative.dev/client/pkg/util"
+)
+
+func TestSourceBinding(t *testing.T) {
+	t.Parallel()
+	test := NewE2eTest(t)
+	test.Setup(t)
+	defer test.Teardown(t)
+
+	test.serviceCreate(t, "testsvc0")
+
+	t.Run("create source binding", func(t *testing.T) {
+		test.sourceBindingCreate(t, "my-binding0", "Deployment:apps/v1:myapp", "svc:testsvc0")
+	})
+
+	t.Run("delete source binding", func(t *testing.T) {
+		test.sourceBindingDelete(t, "my-binding0")
+	})
+
+	t.Run("update source binding", func(t *testing.T) {
+		test.sourceBindingCreate(t, "my-binding1", "Deployment:apps/v1:myapp", "svc:testsvc0")
+		test.serviceCreate(t, "testsvc1")
+		test.sourceBindingUpdate(t, "my-binding1", "Deployment:apps/v1:myapp", "svc:testsvc1")
+		jpSinkRefNameInSpec := "jsonpath={.spec.sink.ref.name}"
+		out, err := test.getResourceFieldsWithJSONPath(t, "sinkbindings.sources.knative.dev", "my-binding1", jpSinkRefNameInSpec)
+		assert.NilError(t, err)
+		assert.Equal(t, out, "testsvc1")
+	})
+
+}
+
+func (test *e2eTest) sourceBindingCreate(t *testing.T, bindingName string, subject string, sink string) {
+	out, err := test.kn.RunWithOpts([]string{"source", "binding", "create", bindingName,
+		"--subject", subject, "--sink", sink}, runOpts{NoNamespace: false})
+	assert.NilError(t, err)
+	assert.Check(t, util.ContainsAllIgnoreCase(out, "Sink", "binding", bindingName, "created", "namespace", test.kn.namespace))
+}
+
+func (test *e2eTest) sourceBindingDelete(t *testing.T, bindingName string) {
+	out, err := test.kn.RunWithOpts([]string{"source", "binding", "delete", bindingName}, runOpts{NoNamespace: false})
+	assert.NilError(t, err)
+	assert.Check(t, util.ContainsAllIgnoreCase(out, "Sink", "binding", bindingName, "deleted", "namespace", test.kn.namespace))
+}
+
+func (test *e2eTest) sourceBindingUpdate(t *testing.T, bindingName string, subject string, sink string) {
+	out, err := test.kn.RunWithOpts([]string{"source", "binding", "update", bindingName,
+		"--subject", subject, "--sink", sink}, runOpts{})
+	assert.NilError(t, err)
+	assert.Check(t, util.ContainsAll(out, bindingName, "updated", "namespace", test.kn.namespace))
+}