refactor: don't use FailNow and SkipNow (#2200)

Signed-off-by: Charles-Edouard Brétéché <[email protected]>
kyverno · Dec 8, 2024 · 0acd1bd · 0acd1bd
1 parent 366c1ac
commit 0acd1bd
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 30 deletions.
diff --git a/pkg/runner/failer/failer.go b/pkg/runner/failer/failer.go
@@ -11,7 +11,6 @@ var defaultFailer = New(false)
 
 type Failer interface {
 	Fail(context.Context)
-	FailNow(context.Context)
 }
 
 type failer struct {
@@ -30,12 +29,6 @@ func (f failer) Fail(ctx context.Context) {
 	t.Fail()
 }
 
-func (f failer) FailNow(ctx context.Context) {
-	f.wait()
-	t := testing.FromContext(ctx)
-	t.FailNow()
-}
-
 func (f failer) wait() {
 	if f.pause {
 		fmt.Println("Failure detected, press ENTER to continue...")
@@ -55,8 +48,3 @@ func Fail(ctx context.Context) {
 	f := getFailerOrDefault(ctx)
 	f.Fail(ctx)
 }
-
-func FailNow(ctx context.Context) {
-	f := getFailerOrDefault(ctx)
-	f.FailNow(ctx)
-}
diff --git a/pkg/runner/processors/step.go b/pkg/runner/processors/step.go
@@ -38,7 +38,7 @@ import (
 )
 
 type StepProcessor interface {
-	Run(context.Context, namespacer.Namespacer, engine.Context)
+	Run(context.Context, namespacer.Namespacer, engine.Context) bool
 }
 
 func NewStepProcessor(
@@ -59,7 +59,7 @@ type stepProcessor struct {
 	basePath string
 }
 
-func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespacer, tc engine.Context) {
+func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespacer, tc engine.Context) bool {
 	t := testing.FromContext(ctx)
 	report := &model.StepReport{
 		Name:      p.step.Name,
@@ -86,7 +86,8 @@ func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespace
 	tc, err := setupContextAndBindings(ctx, tc, contextData, p.step.Bindings...)
 	if err != nil {
 		logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
-		failer.FailNow(ctx)
+		failer.Fail(ctx)
+		return true
 	}
 	cleaner := cleaner.New(tc.Timeouts().Cleanup.Duration, tc.DelayBeforeCleanup(), tc.DeletionPropagation())
 	t.Cleanup(func() {
@@ -195,22 +196,25 @@ func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespace
 		operations, err := p.tryOperation(operationTc.Compilers(), i, namespacer, operationTc.Bindings(), operation, cleaner)
 		if err != nil {
 			logger.Log(logging.Try, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
-			failer.FailNow(ctx)
+			failer.Fail(ctx)
+			return true
 		}
 		for _, operation := range operations {
 			outputs, err := operation.execute(ctx, operationTc, report)
 			if err != nil {
 				if continueOnError {
 					failer.Fail(ctx)
 				} else {
-					failer.FailNow(ctx)
+					failer.Fail(ctx)
+					return true
 				}
 			}
 			for k, v := range outputs {
 				tc = tc.WithBinding(ctx, k, v)
 			}
 		}
 	}
+	return false
 }
 
 func (p *stepProcessor) tryOperation(compilers compilers.Compilers, id int, namespacer namespacer.Namespacer, bindings apis.Bindings, handler v1alpha1.Operation, cleaner cleaner.CleanerCollector) ([]operation, error) {

diff --git a/pkg/runner/processors/step_test.go b/pkg/runner/processors/step_test.go
@@ -37,8 +37,8 @@ func TestStepProcessor_Run(t *testing.T) {
 		basePath               string
 		terminationGracePeriod *metav1.Duration
 		stepSpec               v1alpha1.TestStep
+		want                   bool
 		expectedFail           bool
-		skipped                bool
 	}{{
 		name:   "test with no handler",
 		client: &fake.FakeClient{},
@@ -799,7 +799,8 @@ func TestStepProcessor_Run(t *testing.T) {
 				Error:   &config.Spec.Timeouts.Error,
 				Exec:    &config.Spec.Timeouts.Exec,
 			})
-			stepProcessor.Run(ctx, tc.namespacer, tcontext)
+			got := stepProcessor.Run(ctx, tc.namespacer, tcontext)
+			assert.Equal(t, tc.want, got)
 			if tc.expectedFail {
 				assert.True(t, nt.FailedVar, "expected an error but got none")
 			} else {

diff --git a/pkg/runner/test.go b/pkg/runner/test.go
@@ -88,7 +88,8 @@ func runTest(
 	if err != nil {
 		logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
 		tc.IncFailed()
-		failer.FailNow(ctx)
+		failer.Fail(ctx)
+		return
 	}
 	contextData := processors.ContextData{
 		BasePath:            test.BasePath,
@@ -105,14 +106,17 @@ func runTest(
 	tc, err = processors.SetupContext(ctx, tc, contextData)
 	if err != nil {
 		logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
-		failer.FailNow(ctx)
+		failer.Fail(ctx)
+		return
 	}
 	// skip checks
 	if test.Test.Spec.Skip != nil && *test.Test.Spec.Skip {
-		t.SkipNow()
+		t.Skip()
+		return
 	}
 	if tc.FailFast() && tc.Failed() > 0 {
-		t.SkipNow()
+		t.Skip()
+		return
 	}
 	// setup cleaner
 	mainCleaner := cleaner.New(tc.Timeouts().Cleanup.Duration, nil, tc.DeletionPropagation())
@@ -169,7 +173,8 @@ func runTest(
 		nsTc, namespace, err := processors.SetupNamespace(ctx, tc, namespaceData)
 		if err != nil {
 			logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
-			failer.FailNow(ctx)
+			failer.Fail(ctx)
+			return
 		}
 		tc = nsTc
 		if namespace != nil {
@@ -183,7 +188,8 @@ func runTest(
 	tc, err = processors.SetupBindings(ctx, tc, test.Test.Spec.Bindings...)
 	if err != nil {
 		logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
-		failer.FailNow(ctx)
+		failer.Fail(ctx)
+		return
 	}
 	// run steps
 	for i, step := range test.Test.Spec.Steps {
@@ -197,6 +203,8 @@ func runTest(
 		}
 		tc := tc.WithBinding(ctx, "step", info)
 		processor := processors.NewStepProcessor(step, report, test.BasePath)
-		processor.Run(ctx, nspacer, tc)
+		if stop := processor.Run(ctx, nspacer, tc); stop {
+			return
+		}
 	}
 }
diff --git a/pkg/runner/tests.go b/pkg/runner/tests.go
@@ -39,7 +39,8 @@ func runTests(ctx context.Context, t testing.TTest, clock clock.PassiveClock, ns
 		if err != nil {
 			logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
 			tc.IncFailed()
-			failer.FailNow(ctx)
+			failer.Fail(ctx)
+			return
 		}
 		tc = nsTc
 		if namespace != nil {
@@ -53,7 +54,7 @@ func runTests(ctx context.Context, t testing.TTest, clock clock.PassiveClock, ns
 		if err != nil {
 			logging.Log(ctx, logging.Internal, logging.ErrorStatus, color.BoldRed, logging.ErrSection(err))
 			tc.IncFailed()
-			failer.FailNow(ctx)
+			failer.Fail(ctx)
 		} else {
 			testId := i + 1
 			if len(test.Test.Spec.Scenarios) == 0 {

diff --git a/pkg/testing/context.go b/pkg/testing/context.go
@@ -21,7 +21,6 @@ type TTest interface {
 	Error(args ...any)
 	Errorf(format string, args ...any)
 	Fail()
-	FailNow()
 	Failed() bool
 	Fatal(args ...any)
 	Fatalf(format string, args ...any)
@@ -33,7 +32,6 @@ type TTest interface {
 	Run(name string, f func(t *T)) bool
 	Setenv(key, value string)
 	Skip(args ...any)
-	SkipNow()
 	Skipf(format string, args ...any)
 	Skipped() bool
 	TempDir() string