From c317721afc439e9cc01e64ddb2d96e0a6ec9abfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Chris=20Suszy=C5=84ski?= <ksuszyns@redhat.com>
Date: Tue, 17 Nov 2020 13:00:03 +0100
Subject: [PATCH] Skip at BackgroundOperation's verify stage doesn't hang tests

---
 test/upgrade/functions.go                 | 15 ++++--
 test/upgrade/skip_at_verification_test.go | 66 +++++++++++++++++++++++
 2 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 test/upgrade/skip_at_verification_test.go

diff --git a/test/upgrade/functions.go b/test/upgrade/functions.go
index c6549baf72..50dc637fef 100644
--- a/test/upgrade/functions.go
+++ b/test/upgrade/functions.go
@@ -81,13 +81,10 @@ func NewBackgroundOperation(name string, setup func(c Context),
 // After that happen a handler is invoked to verify environment state and report
 // failures.
 func WaitForStopEvent(bc BackgroundContext, w WaitForStopEventConfiguration) {
-	log := bc.Log
 	for {
 		select {
 		case stopEvent := <-bc.Stop:
-			log.Infof("%s have received a stop event: %s", w.Name, stopEvent.Name())
-			w.OnStop(stopEvent)
-			close(stopEvent.Finished)
+			handleStopEvent(stopEvent, bc, w)
 			return
 		default:
 			w.OnWait(bc, w)
@@ -105,6 +102,16 @@ func (s *StopEvent) Name() string {
 	return s.name
 }
 
+func handleStopEvent(
+	se StopEvent,
+	bc BackgroundContext,
+	wc WaitForStopEventConfiguration,
+) {
+	bc.Log.Infof("%s have received a stop event: %s", wc.Name, se.Name())
+	defer close(se.Finished)
+	wc.OnStop(se)
+}
+
 func enrichSuite(s *Suite) *enrichedSuite {
 	es := &enrichedSuite{
 		installations: s.Installations,
diff --git a/test/upgrade/skip_at_verification_test.go b/test/upgrade/skip_at_verification_test.go
new file mode 100644
index 0000000000..e1086b0916
--- /dev/null
+++ b/test/upgrade/skip_at_verification_test.go
@@ -0,0 +1,66 @@
+/*
+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 implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package upgrade_test
+
+import (
+	"testing"
+
+	"knative.dev/pkg/test/upgrade"
+)
+
+func TestSkipAtBackgroundVerification(t *testing.T) {
+	skipMsg := "It is expected to be skipped"
+	s := upgrade.Suite{
+		Tests: upgrade.Tests{
+			Continual: []upgrade.BackgroundOperation{
+				upgrade.NewBackgroundVerification("ShouldBeSkipped",
+					func(c upgrade.Context) {
+						c.Log.Info("Setup 1")
+					},
+					func(c upgrade.Context) {
+						c.Log.Warn(skipMsg)
+						c.T.Skip(skipMsg)
+					},
+				),
+				upgrade.NewBackgroundVerification("ShouldNotBeSkipped",
+					func(c upgrade.Context) {
+						c.Log.Info("Setup 2")
+					},
+					func(c upgrade.Context) {
+						c.Log.Info("Verify 2")
+					},
+				),
+			},
+		},
+	}
+	log, buf := newExampleZap()
+	s.Execute(upgrade.Configuration{
+		T:   t,
+		Log: log,
+	})
+	out := buf.String()
+	assert := assertions{t: t}
+	assert.textContains(out, texts{elms: []string{
+		upgradeTestRunning,
+		"INFO\tSetup 1",
+		"INFO\tSetup 2",
+		"WARN\t" + skipMsg,
+		"INFO\tVerify 2",
+		"DEBUG\tFinished \"ShouldNotBeSkipped\"",
+		upgradeTestSuccess,
+	}})
+}