From 0acd1bdb3e2712ea9d1bc501383a6e3a7fe4742f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Sun, 8 Dec 2024 23:00:01 +0100 Subject: [PATCH] refactor: don't use FailNow and SkipNow (#2200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- pkg/runner/failer/failer.go | 12 ------------ pkg/runner/processors/step.go | 14 +++++++++----- pkg/runner/processors/step_test.go | 5 +++-- pkg/runner/test.go | 22 +++++++++++++++------- pkg/runner/tests.go | 5 +++-- pkg/testing/context.go | 2 -- 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pkg/runner/failer/failer.go b/pkg/runner/failer/failer.go index 7a2a09833..03a4dc47f 100644 --- 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 index 54bf57a22..f744e5bf8 100644 --- 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,7 +196,8 @@ 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) @@ -203,7 +205,8 @@ func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespace if continueOnError { failer.Fail(ctx) } else { - failer.FailNow(ctx) + failer.Fail(ctx) + return true } } for k, v := range outputs { @@ -211,6 +214,7 @@ func (p *stepProcessor) Run(ctx context.Context, namespacer namespacer.Namespace } } } + 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 index 078e34531..3f0394677 100644 --- 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 index 8d9f71c07..b39964d09 100644 --- 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 index 6bb6710fd..501569f18 100644 --- 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 index 3444ae5a4..91ce13d8a 100644 --- 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