From aa6922c7c501a7f4ba63ba7bb207be0c8296837f Mon Sep 17 00:00:00 2001 From: Christoph Deppisch Date: Thu, 25 Aug 2022 13:00:20 +0200 Subject: [PATCH] fix(#433): Optimize failure() script condition Make sure to run the pre/post script steps that use condition failure() only on those tests that actually have a failed test result instead of evaluating the failure state for the whole test suite. --- pkg/cmd/run.go | 26 ++++++++++++-------------- pkg/cmd/util.go | 30 ++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 1e095757..031f8252 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -198,8 +198,8 @@ func (o *runCmdOptions) runTest(cmd *cobra.Command, source string, results *v1al handleError := func(err error) { handleTestError(runConfig.Config.Namespace.Name, source, results, err) } - defer runSteps(runConfig.Post, runConfig.Config.Namespace.Name, runConfig.BaseDir, results, handleError) - if !runSteps(runConfig.Pre, runConfig.Config.Namespace.Name, runConfig.BaseDir, results, handleError) { + defer runSteps(runConfig.Post, runConfig.Config.Namespace.Name, runConfig.BaseDir, source, results, handleError) + if !runSteps(runConfig.Pre, runConfig.Config.Namespace.Name, runConfig.BaseDir, source, results, handleError) { return } @@ -254,8 +254,8 @@ func (o *runCmdOptions) runTestGroup(cmd *cobra.Command, source string, results handleError := func(err error) { handleTestError(runConfig.Config.Namespace.Name, source, results, err) } - defer runSteps(runConfig.Post, runConfig.Config.Namespace.Name, runConfig.BaseDir, results, handleError) - if !runSteps(runConfig.Pre, runConfig.Config.Namespace.Name, runConfig.BaseDir, results, handleError) { + defer runSteps(runConfig.Post, runConfig.Config.Namespace.Name, runConfig.BaseDir, source, results, handleError) + if !runSteps(runConfig.Pre, runConfig.Config.Namespace.Name, runConfig.BaseDir, source, results, handleError) { return } @@ -352,7 +352,7 @@ func (o *runCmdOptions) createTempNamespace(runConfig *config.RunConfig, cmd *co instance, err = v1alpha1.FindGlobalInstance(o.Context, c) if err != nil && k8serrors.IsForbidden(err) { - // not allowed to list all instances on the clusterr + // not allowed to list all instances on the cluster return namespace, nil } else if err != nil { return namespace, err @@ -548,9 +548,7 @@ func (o *runCmdOptions) createAndRunTest(ctx context.Context, c client.Client, c } if runConfig.Config.Dump.Enabled { - if runConfig.Config.Dump.FailedOnly && - test.Status.Phase != v1alpha1.TestPhaseFailed && test.Status.Phase != v1alpha1.TestPhaseError && - len(test.Status.Errors) == 0 && !hasSuiteErrors(&test.Status.Results) { + if runConfig.Config.Dump.FailedOnly && !isFailed(&test) { fmt.Println("Skip dump for successful test") } else { var fileName string @@ -766,13 +764,13 @@ func (o *runCmdOptions) newSettings(ctx context.Context, runConfig *config.RunCo return nil, nil } -func runSteps(steps []config.StepConfig, namespace, baseDir string, results *v1alpha1.TestResults, handleError func(err error)) bool { +func runSteps(steps []config.StepConfig, namespace, baseDir string, name string, results *v1alpha1.TestResults, handleError func(err error)) bool { for idx, step := range steps { if len(step.Name) == 0 { step.Name = fmt.Sprintf("step-%d", idx) } - if skipStep(step, results) { + if skipStep(step, name, results) { fmt.Printf("Skip %s\n", step.Name) continue } @@ -782,7 +780,7 @@ func runSteps(steps []config.StepConfig, namespace, baseDir string, results *v1a if desc == "" { desc = fmt.Sprintf("script %s", step.Script) } - if err := runScript(step.Script, desc, namespace, baseDir, hasErrors(results), step.Timeout); err != nil { + if err := runScript(step.Script, desc, namespace, baseDir, hasError(name, results), step.Timeout); err != nil { handleError(fmt.Errorf(fmt.Sprintf("Failed to run %s: %v", desc, err))) return false } @@ -824,7 +822,7 @@ func runSteps(steps []config.StepConfig, namespace, baseDir string, results *v1a if desc == "" { desc = fmt.Sprintf("inline command %d", idx) } - if err := runScript(file.Name(), desc, namespace, baseDir, hasErrors(results), step.Timeout); err != nil { + if err := runScript(file.Name(), desc, namespace, baseDir, hasError(name, results), step.Timeout); err != nil { handleError(fmt.Errorf(fmt.Sprintf("Failed to run %s: %v", desc, err))) return false } @@ -834,7 +832,7 @@ func runSteps(steps []config.StepConfig, namespace, baseDir string, results *v1a return true } -func skipStep(step config.StepConfig, results *v1alpha1.TestResults) bool { +func skipStep(step config.StepConfig, name string, results *v1alpha1.TestResults) bool { if step.If == "" { return false } @@ -867,7 +865,7 @@ func skipStep(step config.StepConfig, results *v1alpha1.TestResults) bool { case "os": skipStep = (keyValue)[1] != r.GOOS case "failure()": - skipStep = !hasErrors(results) + skipStep = !hasError(name, results) } if skipStep { diff --git a/pkg/cmd/util.go b/pkg/cmd/util.go index 7f1fae2d..ef4af024 100644 --- a/pkg/cmd/util.go +++ b/pkg/cmd/util.go @@ -33,6 +33,7 @@ import ( "github.com/citrusframework/yaks/pkg/apis/yaks/v1alpha1" "github.com/citrusframework/yaks/pkg/cmd/config" + "github.com/citrusframework/yaks/pkg/util/kubernetes" p "github.com/gertd/go-pluralize" "github.com/mitchellh/mapstructure" "github.com/spf13/cobra" @@ -230,8 +231,18 @@ func resolvePath(runConfig *config.RunConfig, resource string) string { } func hasErrors(results *v1alpha1.TestResults) bool { - for i := range results.Suites { - if hasSuiteErrors(&results.Suites[i]) { + for _, suite := range results.Suites { + if hasSuiteErrors(&suite) { + return true + } + } + + return false +} + +func hasError(name string, results *v1alpha1.TestResults) bool { + for _, suite := range results.Suites { + if hasTestError(name, &suite) { return true } } @@ -240,13 +251,24 @@ func hasErrors(results *v1alpha1.TestResults) bool { } func hasSuiteErrors(suite *v1alpha1.TestSuite) bool { - if len(suite.Errors) > 0 || suite.Summary.Errors > 0 || suite.Summary.Failed > 0 { - return true + return len(suite.Errors) > 0 || suite.Summary.Errors > 0 || suite.Summary.Failed > 0 +} + +func hasTestError(name string, suite *v1alpha1.TestSuite) bool { + for _, test := range suite.Tests { + if test.Name == kubernetes.SanitizeName(name) { + return test.ErrorType != "" || test.ErrorMessage != "" + } } return false } +func isFailed(test *v1alpha1.Test) bool { + return test.Status.Phase == v1alpha1.TestPhaseFailed || + test.Status.Phase == v1alpha1.TestPhaseError && len(test.Status.Errors) > 0 +} + func loadData(ctx context.Context, fileName string) (string, error) { var content []byte var err error