Repository: knative/client
Pull request #661 (2020-02-12 19:18:10)

fix(service wait): Wrong check for error window
The error window introduced with #644 had a wrong conditional. Fixed that and added a test which would have detected this.

Also, this should fix some issues which we tried to detect with #659.

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/661#" title="Author self-approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/master/OWNERS)~~ [rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-client-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[pkg/wait/wait_for_ready.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_client/661/pull-knative-client-go-coverage/1227673301921304577/artifacts/line-cov.html#file0) | 66.7% | 74.3% | 7.6

rhuss: @navidshaikh fix for a stupid error. btw, I'm not sure anymore whether the original flake really was a race in reconciliation, it looks more like an optimistic lock error. See https://github.com/knative/serving/issues/6784 for tracking this.
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/client/pull/661#pullrequestreview-357791112" title="Approved">maximilien</a>*, *<a href="https://github.com/knative/client/pull/661#" title="Author self-approved">rhuss</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fclient).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative/client/blob/master/OWNERS)~~ [maximilien,rhuss]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/kn/commands/flags/sink.go b/pkg/kn/commands/flags/sink.go
index ba442ae3..2a6ce98f 100644
--- a/pkg/kn/commands/flags/sink.go
+++ b/pkg/kn/commands/flags/sink.go
@@ -105,3 +105,18 @@ func parseSink(sink string) (string, string) {
 		return parts[0], parts[1]
 	}
 }
+
+// SinkToString prepares a sink for list output
+func SinkToString(sink duckv1.Destination) string {
+	if sink.Ref != nil {
+		if sink.Ref.Kind == "Service" {
+			return fmt.Sprintf("svc:%s", sink.Ref.Name)
+		} else {
+			return fmt.Sprintf("%s:%s", strings.ToLower(sink.Ref.Kind), sink.Ref.Name)
+		}
+	}
+	if sink.URI != nil {
+		return sink.URI.String()
+	}
+	return ""
+}
diff --git a/pkg/kn/commands/source/apiserver/flags.go b/pkg/kn/commands/source/apiserver/flags.go
index 98f1cd33..0815214c 100644
--- a/pkg/kn/commands/source/apiserver/flags.go
+++ b/pkg/kn/commands/source/apiserver/flags.go
@@ -165,6 +165,7 @@ func APIServerSourceListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of the ApiServer source", Priority: 1},
 		{Name: "Resources", Type: "string", Description: "Event resources configured for the ApiServer source", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink of the ApiServer source", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the ApiServer source", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the ApiServer source", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -180,6 +181,7 @@ func printSource(source *v1alpha1.ApiServerSource, options hprinters.PrintOption
 	}
 
 	name := source.Name
+	age := commands.TranslateTimestampSince(source.CreationTimestamp)
 	conditions := commands.ConditionsValue(source.Status.Conditions)
 	ready := commands.ReadyCondition(source.Status.Conditions)
 	reason := strings.TrimSpace(commands.NonReadyConditionReason(source.Status.Conditions))
@@ -206,7 +208,7 @@ func printSource(source *v1alpha1.ApiServerSource, options hprinters.PrintOption
 		row.Cells = append(row.Cells, source.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, strings.Join(resources[:], ","), sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, strings.Join(resources[:], ","), sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/source/binding/binding.go b/pkg/kn/commands/source/binding/binding.go
index 34c0ee00..b20e0c0d 100644
--- a/pkg/kn/commands/source/binding/binding.go
+++ b/pkg/kn/commands/source/binding/binding.go
@@ -23,7 +23,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/clientcmd"
 	"knative.dev/eventing/pkg/client/clientset/versioned/typed/sources/v1alpha1"
-	duckv1 "knative.dev/pkg/apis/duck/v1"
 	"knative.dev/pkg/tracker"
 
 	"knative.dev/client/pkg/kn/commands"
@@ -133,23 +132,6 @@ func subjectToString(ref tracker.Reference) string {
 	return ret
 }
 
-// sinkToString prepares a sink for list output
-// This is kept here until we have moved everything to duckv1 (currently the other sources
-// are still on duckv1beta1)
-func sinkToString(sink duckv1.Destination) string {
-	if sink.Ref != nil {
-		if sink.Ref.Kind == "Service" {
-			return fmt.Sprintf("svc:%s", sink.Ref.Name)
-		} else {
-			return fmt.Sprintf("%s:%s", sink.Ref.Kind, sink.Ref.Name)
-		}
-	}
-	if sink.URI != nil {
-		return sink.URI.String()
-	}
-	return ""
-}
-
 // updateCeOverrides updates the values of the --ce-override flags if given
 func updateCeOverrides(bindingFlags bindingUpdateFlags, bindingBuilder *clientsourcesv1alpha1.SinkBindingBuilder) error {
 	if bindingFlags.ceOverrides != nil {
diff --git a/pkg/kn/commands/source/binding/flags.go b/pkg/kn/commands/source/binding/flags.go
index 12ef776e..e8b38884 100644
--- a/pkg/kn/commands/source/binding/flags.go
+++ b/pkg/kn/commands/source/binding/flags.go
@@ -20,6 +20,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/kn/commands/flags"
 	hprinters "knative.dev/client/pkg/printers"
 
 	"knative.dev/eventing/pkg/apis/sources/v1alpha1"
@@ -41,6 +42,7 @@ func BindingListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of sink binding", Priority: 1},
 		{Name: "Subject", Type: "string", Description: "Subject part of binding", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink part of binding", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of binding", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the sink binding", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -57,7 +59,8 @@ func printSinkBinding(binding *v1alpha1.SinkBinding, options hprinters.PrintOpti
 
 	name := binding.Name
 	subject := subjectToString(binding.Spec.Subject)
-	sink := sinkToString(binding.Spec.Sink)
+	sink := flags.SinkToString(binding.Spec.Sink)
+	age := commands.TranslateTimestampSince(binding.CreationTimestamp)
 	conditions := commands.ConditionsValue(binding.Status.Conditions)
 	ready := commands.ReadyCondition(binding.Status.Conditions)
 	reason := commands.NonReadyConditionReason(binding.Status.Conditions)
@@ -66,7 +69,7 @@ func printSinkBinding(binding *v1alpha1.SinkBinding, options hprinters.PrintOpti
 		row.Cells = append(row.Cells, binding.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, subject, sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, subject, sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/source/cronjob/flags.go b/pkg/kn/commands/source/cronjob/flags.go
index 564a7c39..cb100fb0 100644
--- a/pkg/kn/commands/source/cronjob/flags.go
+++ b/pkg/kn/commands/source/cronjob/flags.go
@@ -45,6 +45,7 @@ func CronJobSourceListHandlers(h hprinters.PrintHandler) {
 		{Name: "Name", Type: "string", Description: "Name of the CronJob source", Priority: 1},
 		{Name: "Schedule", Type: "string", Description: "Schedule of the CronJob source", Priority: 1},
 		{Name: "Sink", Type: "string", Description: "Sink of the CronJob source", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the CronJob source", Priority: 1},
 		{Name: "Conditions", Type: "string", Description: "Ready state conditions", Priority: 1},
 		{Name: "Ready", Type: "string", Description: "Ready state of the CronJob source", Priority: 1},
 		{Name: "Reason", Type: "string", Description: "Reason if state is not Ready", Priority: 1},
@@ -61,6 +62,7 @@ func printSource(source *v1alpha1.CronJobSource, options hprinters.PrintOptions)
 
 	name := source.Name
 	schedule := source.Spec.Schedule
+	age := commands.TranslateTimestampSince(source.CreationTimestamp)
 	conditions := commands.ConditionsValue(source.Status.Conditions)
 	ready := commands.ReadyCondition(source.Status.Conditions)
 	reason := commands.NonReadyConditionReason(source.Status.Conditions)
@@ -83,7 +85,7 @@ func printSource(source *v1alpha1.CronJobSource, options hprinters.PrintOptions)
 		row.Cells = append(row.Cells, source.Namespace)
 	}
 
-	row.Cells = append(row.Cells, name, schedule, sink, conditions, ready, reason)
+	row.Cells = append(row.Cells, name, schedule, sink, age, conditions, ready, reason)
 	return []metav1beta1.TableRow{row}, nil
 }
 
diff --git a/pkg/kn/commands/source/human_readable_flags.go b/pkg/kn/commands/source/human_readable_flags.go
index 11f8b917..e7548519 100644
--- a/pkg/kn/commands/source/human_readable_flags.go
+++ b/pkg/kn/commands/source/human_readable_flags.go
@@ -26,10 +26,11 @@ import (
 )
 
 var sourceTypeDescription = map[string]string{
-	"ApiServerSource": "Kubernetes API Server events source",
-	"ContainerSource": "Container events source",
-	"CronJobSource":   "CronJob events source",
-	"SinkBinding":     "Binding Pattern for ContainerSource",
+	"ApiServerSource": "Watch and send Kubernetes API events to a sink",
+	"ContainerSource": "Connect a custom container image to a sink",
+	"CronJobSource":   "Send periodically constant data to a sink",
+	"SinkBinding":     "Binding for connecting a PodSpecable to a sink",
+	"PingSource":      "Send periodically ping events to a sink",
 }
 
 func getSourceTypeDescription(kind string) string {
diff --git a/pkg/kn/commands/trigger/list_flags.go b/pkg/kn/commands/trigger/list_flags.go
index 3c65041d..741ea056 100644
--- a/pkg/kn/commands/trigger/list_flags.go
+++ b/pkg/kn/commands/trigger/list_flags.go
@@ -20,6 +20,7 @@ import (
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/kn/commands/flags"
 	hprinters "knative.dev/client/pkg/printers"
 	eventing "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 )
@@ -27,12 +28,14 @@ import (
 // TriggerListHandlers handles printing human readable table for `kn source list-types` command's output
 func TriggerListHandlers(h hprinters.PrintHandler) {
 	sourceTypesColumnDefinitions := []metav1beta1.TableColumnDefinition{
-		{Name: "Namespace", Type: "string", Description: "Namespace of the trigger", Priority: 0},
-		{Name: "Name", Type: "string", Description: "Name of the trigger", Priority: 1},
-		{Name: "Broker", Type: "string", Description: "Name of the broker", Priority: 1},
-		{Name: "Subscriber_URI", Type: "string", Description: "URI of the subscriber", Priority: 1},
-		{Name: "Ready", Type: "string", Description: "Ready condition status of the trigger", Priority: 1},
-		{Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the trigger", Priority: 1},
+		{Name: "Namespace", Type: "string", Description: "Namespace of the trigger.", Priority: 0},
+		{Name: "Name", Type: "string", Description: "Name of the trigger.", Priority: 1},
+		{Name: "Broker", Type: "string", Description: "Name of the broker.", Priority: 1},
+		{Name: "Sink", Type: "string", Description: "Sink for events, i.e. the subscriber.", Priority: 1},
+		{Name: "Age", Type: "string", Description: "Age of the trigger.", Priority: 1},
+		{Name: "Conditions", Type: "string", Description: "Ready state conditions.", Priority: 1},
+		{Name: "Ready", Type: "string", Description: "Ready condition status of the trigger.", Priority: 1},
+		{Name: "Reason", Type: "string", Description: "Reason for non-ready condition of the trigger.", Priority: 1},
 	}
 	h.TableHandler(sourceTypesColumnDefinitions, printTrigger)
 	h.TableHandler(sourceTypesColumnDefinitions, printTriggerList)
@@ -42,7 +45,9 @@ func TriggerListHandlers(h hprinters.PrintHandler) {
 func printTrigger(trigger *eventing.Trigger, options hprinters.PrintOptions) ([]metav1beta1.TableRow, error) {
 	name := trigger.Name
 	broker := trigger.Spec.Broker
-	uri := trigger.Status.SubscriberURI
+	sink := flags.SinkToString(trigger.Spec.Subscriber)
+	age := commands.TranslateTimestampSince(trigger.CreationTimestamp)
+	conditions := commands.ConditionsValue(trigger.Status.Conditions)
 	ready := commands.ReadyCondition(trigger.Status.Conditions)
 	reason := commands.NonReadyConditionReason(trigger.Status.Conditions)
 
@@ -57,7 +62,9 @@ func printTrigger(trigger *eventing.Trigger, options hprinters.PrintOptions) ([]
 	row.Cells = append(row.Cells,
 		name,
 		broker,
-		uri,
+		sink,
+		age,
+		conditions,
 		ready,
 		reason)
 	return []metav1beta1.TableRow{row}, nil
diff --git a/pkg/kn/core/root.go b/pkg/kn/core/root.go
index c6ea75d8..4f5700cd 100644
--- a/pkg/kn/core/root.go
+++ b/pkg/kn/core/root.go
@@ -152,6 +152,9 @@ func NewKnCommand(params ...commands.KnParams) *cobra.Command {
 	rootCmd.AddCommand(source.NewSourceCommand(p))
 	rootCmd.AddCommand(trigger.NewTriggerCommand(p))
 
+	// Initialize default `help` cmd early to prevent unknown command errors
+	rootCmd.InitDefaultHelpCmd()
+
 	// Deal with empty and unknown sub command groups
 	EmptyAndUnknownSubCommands(rootCmd)
 
diff --git a/pkg/wait/wait_for_ready.go b/pkg/wait/wait_for_ready.go
index d5a5c563..e4c51776 100644
--- a/pkg/wait/wait_for_ready.go
+++ b/pkg/wait/wait_for_ready.go
@@ -41,7 +41,8 @@ type waitForReadyConfig struct {
 type WaitForReady interface {
 
 	// Wait on resource the resource with this name until a given timeout
-	// and write event messages for unknown event to the status writer
+	// and write event messages for unknown event to the status writer.
+	// Returns an error (if any) and the overall time it took to wait
 	Wait(name string, timeout time.Duration, msgCallback MessageCallback) (error, time.Duration)
 }
 
@@ -161,9 +162,10 @@ func (w *waitForReadyConfig) waitForReadyCondition(start time.Time, name string,
 						// 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 {
+						if errorTimer == nil {
+							err := fmt.Errorf("%s: %s", cond.Reason, cond.Message)
 							errorTimer = time.AfterFunc(errorWindow, func() {
-								errChan <- fmt.Errorf("%s: %s", cond.Reason, cond.Message)
+								errChan <- err
 							})
 						}
 					}


Test patch:
diff --git a/pkg/kn/commands/source/apiserver/list_test.go b/pkg/kn/commands/source/apiserver/list_test.go
index ea3ec354..652563f6 100644
--- a/pkg/kn/commands/source/apiserver/list_test.go
+++ b/pkg/kn/commands/source/apiserver/list_test.go
@@ -36,7 +36,7 @@ func TestListAPIServerSource(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "sources should be listed")
-	util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testsource", "Eventing:v1:false", "mysvc")
 
 	apiServerRecorder.Validate()
@@ -52,7 +52,7 @@ func TestListAPIServerSourceEmpty(t *testing.T) {
 
 	out, err := executeAPIServerSourceCommand(apiServerClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "RESOURCES", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "ApiServer", "source", "found")
 
 	apiServerRecorder.Validate()
diff --git a/pkg/kn/commands/source/binding/list_test.go b/pkg/kn/commands/source/binding/list_test.go
index a8415eac..7367bea6 100644
--- a/pkg/kn/commands/source/binding/list_test.go
+++ b/pkg/kn/commands/source/binding/list_test.go
@@ -38,7 +38,7 @@ func TestListSimple(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsAll(out, "NAME", "SUBJECT", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "SUBJECT", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testbinding", "deployment:apps/v1:mydeploy", "mysvc")
 
 	bindingRecorder.Validate()
@@ -53,7 +53,7 @@ func TestListEmpty(t *testing.T) {
 
 	out, err := executeSinkBindingCommand(bindingClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "SUBJECT", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "SUBJECT", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "sink binding", "found")
 
 	bindingRecorder.Validate()
diff --git a/pkg/kn/commands/source/cronjob/list_test.go b/pkg/kn/commands/source/cronjob/list_test.go
index 9015ba13..a99a3ec7 100644
--- a/pkg/kn/commands/source/cronjob/list_test.go
+++ b/pkg/kn/commands/source/cronjob/list_test.go
@@ -36,7 +36,7 @@ func TestListCronJobSource(t *testing.T) {
 
 	out, err := executeCronJobSourceCommand(cronjobClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsAll(out, "NAME", "SCHEDULE", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsAll(out, "NAME", "SCHEDULE", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "testsource", "* * * * */2", "mysvc")
 
 	cronJobRecorder.Validate()
@@ -52,7 +52,7 @@ func TestListCronJobSourceEmpty(t *testing.T) {
 
 	out, err := executeCronJobSourceCommand(cronjobClient, nil, "list")
 	assert.NilError(t, err, "Sources should be listed")
-	util.ContainsNone(out, "NAME", "SCHEDULE", "SINK", "CONDITIONS", "READY", "REASON")
+	util.ContainsNone(out, "NAME", "SCHEDULE", "SINK", "AGE", "CONDITIONS", "READY", "REASON")
 	util.ContainsAll(out, "No", "CronJob", "source", "found")
 
 	cronJobRecorder.Validate()
diff --git a/pkg/kn/commands/trigger/list_test.go b/pkg/kn/commands/trigger/list_test.go
index 5c290162..b4e735c6 100644
--- a/pkg/kn/commands/trigger/list_test.go
+++ b/pkg/kn/commands/trigger/list_test.go
@@ -49,7 +49,7 @@ func TestTriggerList(t *testing.T) {
 	assert.NilError(t, err)
 
 	outputLines := strings.Split(output, "\n")
-	assert.Check(t, util.ContainsAll(outputLines[0], "NAME", "BROKER", "SUBSCRIBER_URI", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(outputLines[0], "NAME", "BROKER", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(outputLines[1], "trigger1", "mybroker1", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[2], "trigger2", "mybroker2", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[3], "trigger3", "mybroker3", "mysink"))
@@ -90,7 +90,7 @@ func TestTriggerListAllNamespace(t *testing.T) {
 	assert.NilError(t, err)
 
 	outputLines := strings.Split(output, "\n")
-	assert.Check(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "BROKER", "SUBSCRIBER_URI", "READY", "REASON"))
+	assert.Check(t, util.ContainsAll(outputLines[0], "NAMESPACE", "NAME", "BROKER", "SINK", "AGE", "CONDITIONS", "READY", "REASON"))
 	assert.Check(t, util.ContainsAll(outputLines[1], "default1", "trigger1", "mybroker1", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[2], "default2", "trigger2", "mybroker2", "mysink"))
 	assert.Check(t, util.ContainsAll(outputLines[3], "default3", "trigger3", "mybroker3", "mysink"))
diff --git a/pkg/wait/wait_for_ready_test.go b/pkg/wait/wait_for_ready_test.go
index ad80ad19..6ff89d8c 100644
--- a/pkg/wait/wait_for_ready_test.go
+++ b/pkg/wait/wait_for_ready_test.go
@@ -30,7 +30,7 @@ import (
 type waitForReadyTestCase struct {
 	events           []watch.Event
 	timeout          time.Duration
-	errorExpected    bool
+	errorText        string
 	messagesExpected []string
 }
 
@@ -54,12 +54,16 @@ func TestAddWaitForReady(t *testing.T) {
 		})
 		close(fakeWatchApi.eventChan)
 
-		if !tc.errorExpected && err != nil {
+		if tc.errorText == "" && err != nil {
 			t.Errorf("%d: Error received %v", i, err)
 			continue
 		}
-		if tc.errorExpected && err == nil {
-			t.Errorf("%d: No error but expected one", i)
+		if tc.errorText != "" {
+			if err == nil {
+				t.Errorf("%d: No error but expected one", i)
+			} else {
+				assert.ErrorContains(t, err, tc.errorText)
+			}
 		}
 
 		// check messages
@@ -75,20 +79,34 @@ func TestAddWaitForReady(t *testing.T) {
 // Test cases which consists of a series of events to send and the expected behaviour.
 func prepareTestCases(name string) []waitForReadyTestCase {
 	return []waitForReadyTestCase{
-		tc(peNormal, name, false),
-		tc(peError, name, true),
-		tc(peWrongGeneration, name, true),
-		tc(peTimeout, name, true),
-		tc(peReadyFalseWithinErrorWindow, name, false),
+		errorTest(name),
+		tc(peNormal, name, time.Second, ""),
+		tc(peWrongGeneration, name, 1*time.Second, "timeout"),
+		tc(peTimeout, name, time.Second, "timeout"),
+		tc(peReadyFalseWithinErrorWindow, name, time.Second, ""),
 	}
 }
 
-func tc(f func(name string) (evts []watch.Event, nrMessages int), name string, isError bool) waitForReadyTestCase {
+func errorTest(name string) waitForReadyTestCase {
+	events := []watch.Event{
+		{watch.Added, CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionUnknown, "", "msg1")},
+		{watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionFalse, corev1.ConditionTrue, "FakeError", "Test Error")},
+	}
+
+	return waitForReadyTestCase{
+		events:           events,
+		timeout:          3 * time.Second,
+		errorText:        "FakeError",
+		messagesExpected: []string{"msg1", "Test Error"},
+	}
+}
+
+func tc(f func(name string) (evts []watch.Event, nrMessages int), name string, timeout time.Duration, errorTxt string) waitForReadyTestCase {
 	events, nrMsgs := f(name)
 	return waitForReadyTestCase{
 		events,
-		time.Second,
-		isError,
+		timeout,
+		errorTxt,
 		pMessages(nrMsgs),
 	}
 }
@@ -110,14 +128,6 @@ func peNormal(name string) ([]watch.Event, int) {
 	}, len(messages)
 }
 
-func peError(name string) ([]watch.Event, int) {
-	messages := pMessages(1)
-	return []watch.Event{
-		{watch.Added, CreateTestServiceWithConditions(name, corev1.ConditionUnknown, corev1.ConditionUnknown, "", messages[0])},
-		{watch.Modified, CreateTestServiceWithConditions(name, corev1.ConditionFalse, corev1.ConditionTrue, "FakeError", "")},
-	}, len(messages)
-}
-
 func peTimeout(name string) ([]watch.Event, int) {
 	messages := pMessages(1)
 	return []watch.Event{