Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] fix(#433): Optimize failure() script condition #434

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[WIP] fix(#433): Optimize failure() script condition #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions pkg/cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -766,13 +764,24 @@ 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 {
fileNames := make([]string, 0)

defer func() {
for _, fileName := range fileNames {
err := os.Remove(fileName)
if err != nil {
handleError(fmt.Errorf(fmt.Sprintf("Failed to remove temporary script file %s: %v", fileName, err)))
}
}
}()

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
}
Expand All @@ -782,7 +791,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
}
Expand All @@ -795,7 +804,7 @@ func runSteps(steps []config.StepConfig, namespace, baseDir string, results *v1a
handleError(err)
return false
}
defer os.Remove(file.Name())
fileNames = append(fileNames, file.Name())

_, err = file.WriteString("#!/bin/bash\n\nset -e\n\n")
if err != nil {
Expand Down Expand Up @@ -824,7 +833,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
}
Expand All @@ -834,7 +843,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
}
Expand Down Expand Up @@ -867,7 +876,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 {
Expand Down
37 changes: 33 additions & 4 deletions pkg/cmd/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestStepOsCheck(t *testing.T) {
saveErr := func(err error) {
scriptError = err
}
runSteps(steps, "default", "", &v1alpha1.TestResults{}, saveErr)
runSteps(steps, "default", "", "foo", &v1alpha1.TestResults{}, saveErr)

assert.NilError(t, scriptError)
}
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestStepEnvCheck(t *testing.T) {
saveErr := func(err error) {
scriptError = err
}
runSteps(steps, "default", "", &v1alpha1.TestResults{}, saveErr)
runSteps(steps, "default", "", "foo", &v1alpha1.TestResults{}, saveErr)

assert.NilError(t, scriptError)
}
Expand Down Expand Up @@ -117,7 +117,7 @@ func TestStepCheckCombinations(t *testing.T) {
saveErr := func(err error) {
scriptError = err
}
runSteps(steps, "default", "", &v1alpha1.TestResults{}, saveErr)
runSteps(steps, "default", "", "foo", &v1alpha1.TestResults{}, saveErr)

assert.NilError(t, scriptError)
}
Expand Down Expand Up @@ -146,7 +146,36 @@ func TestStepOnFailure(t *testing.T) {
saveErr := func(err error) {
scriptError = err
}
runSteps(steps, "default", "", &v1alpha1.TestResults{}, saveErr)

results := v1alpha1.TestResults{
Suites: []v1alpha1.TestSuite{
{
Tests: []v1alpha1.TestResult{
{
Name: "foo",
ErrorType: "",
ErrorMessage: "",
},
},
},
},
}

runSteps(steps, "default", "", "foo", &results, saveErr)

assert.NilError(t, scriptError)

results.Suites[0].Tests = append(results.Suites[0].Tests, v1alpha1.TestResult{
Name: "bar",
ErrorType: "error",
ErrorMessage: "Something went wrong",
})

runSteps(steps, "default", "", "foo", &results, saveErr)

assert.NilError(t, scriptError)

runSteps(steps, "default", "", "bar", &results, saveErr)

assert.Equal(t, scriptError.Error(), "Failed to run failure-check-fail: exit status 127")
}
20 changes: 15 additions & 5 deletions pkg/cmd/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -230,23 +231,32 @@ 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 len(suite.Errors) > 0 || suite.Summary.Errors > 0 || suite.Summary.Failed > 0 {
return true
}
}

return false
}

func hasSuiteErrors(suite *v1alpha1.TestSuite) bool {
if len(suite.Errors) > 0 || suite.Summary.Errors > 0 || suite.Summary.Failed > 0 {
return true
func hasError(name string, results *v1alpha1.TestResults) bool {
for _, suite := range results.Suites {
for _, test := range suite.Tests {
if test.Name == kubernetes.SanitizeName(name) && (test.ErrorType != "" || test.ErrorMessage != "") {
return true
}
}
}

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
Expand Down