From 1891c72ab158508e36009d16b24913fa5836422b Mon Sep 17 00:00:00 2001 From: Markus Wolf Date: Wed, 8 Dec 2021 21:57:42 +0100 Subject: [PATCH] fix: continue jobs + steps after failure (#840) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: continue jobs + steps after failure To allow proper if expression handling on jobs and steps (like always, success, failure, ...) we need to continue running all executors in the prepared chain. To keep the error handling intact we add an occurred error to the go context and handle it later in the pipeline/chain. Also we add the job result to the needs context to give expressions access to it. The needs object, failure and success functions are split between run context (on jobs) and step context. Closes #442 Co-authored-by: Björn Brauer * style: correct linter warnings Co-authored-by: Björn Brauer * fix: job if value defaults to success() As described in the documentation, a default value of "success()" is applied when no "if" value is present on the job. https://docs.github.com/en/actions/learn-github-actions/expressions#job-status-check-functions Co-authored-by: Markus Wolf * fix: check job needs recursively Ensure job result includes results of previous jobs Co-authored-by: Markus Wolf * test: add runner test for job status check functions Co-authored-by: Markus Wolf * test: add unit tests for run context if evaluation Co-authored-by: Björn Brauer * refactor: move if expression evaluation Move if expression evaluation into own function (step context) to better support unit testing. Co-authored-by: Björn Brauer * test: add unit tests for step context if evaluation Co-authored-by: Markus Wolf * fix: handle job error more resilient The job error is not stored in a context map instead of a context added value. Since context values are immutable an added value requires to keep the new context in all cases. This is fragile since it might slip unnoticed to other parts of the code. Storing the error of a job in the context map will make it more stable, since the map is always there and the context of the pipeline is stable for the whole run. * feat: steps should use a default if expression of success() * test: add integration test for if-expressions * chore: disable editorconfig-checker for yaml multiline string Co-authored-by: Björn Brauer Co-authored-by: Björn Brauer --- pkg/common/executor.go | 9 +- pkg/common/job_error.go | 30 ++++ pkg/model/planner.go | 3 + pkg/model/workflow.go | 4 + pkg/runner/expression.go | 145 +++++++++++++++++- pkg/runner/run_context.go | 82 ++++------ pkg/runner/run_context_test.go | 141 ++++++++++++++++- pkg/runner/runner.go | 17 +- pkg/runner/runner_test.go | 2 + pkg/runner/step_context.go | 15 ++ pkg/runner/step_context_test.go | 87 +++++++++++ pkg/runner/testdata/if-expressions/push.yml | 29 ++++ pkg/runner/testdata/job-status-check/push.yml | 28 ++++ 13 files changed, 523 insertions(+), 69 deletions(-) create mode 100644 pkg/common/job_error.go mode change 100755 => 100644 pkg/runner/run_context.go create mode 100644 pkg/runner/testdata/if-expressions/push.yml create mode 100644 pkg/runner/testdata/job-status-check/push.yml diff --git a/pkg/common/executor.go b/pkg/common/executor.go index cd92a6c5..a7eec9e1 100644 --- a/pkg/common/executor.go +++ b/pkg/common/executor.go @@ -136,13 +136,12 @@ func (e Executor) Then(then Executor) Executor { case Warning: log.Warning(err.Error()) default: - log.Debugf("%+v", err) - return err + SetJobError(ctx, err) } + } else if ctx.Err() != nil { + SetJobError(ctx, ctx.Err()) } - if ctx.Err() != nil { - return ctx.Err() - } + return then(ctx) } } diff --git a/pkg/common/job_error.go b/pkg/common/job_error.go new file mode 100644 index 00000000..334c6ca7 --- /dev/null +++ b/pkg/common/job_error.go @@ -0,0 +1,30 @@ +package common + +import ( + "context" +) + +type jobErrorContextKey string + +const jobErrorContextKeyVal = jobErrorContextKey("job.error") + +// JobError returns the job error for current context if any +func JobError(ctx context.Context) error { + val := ctx.Value(jobErrorContextKeyVal) + if val != nil { + if container, ok := val.(map[string]error); ok { + return container["error"] + } + } + return nil +} + +func SetJobError(ctx context.Context, err error) { + ctx.Value(jobErrorContextKeyVal).(map[string]error)["error"] = err +} + +// WithJobErrorContainer adds a value to the context as a container for an error +func WithJobErrorContainer(ctx context.Context) context.Context { + container := map[string]error{} + return context.WithValue(ctx, jobErrorContextKeyVal, container) +} diff --git a/pkg/model/planner.go b/pkg/model/planner.go index 5cb2e4d8..d38e8363 100644 --- a/pkg/model/planner.go +++ b/pkg/model/planner.go @@ -78,6 +78,9 @@ func FixIfStatement(content []byte, wr *Workflow) error { if err != nil { return err } + if val == "" { + val = "success()" + } jobs[j].Steps[i].If.Value = val } } diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index eb6e4865..bf06d3a3 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -69,6 +69,7 @@ type Job struct { RawContainer yaml.Node `yaml:"container"` Defaults Defaults `yaml:"defaults"` Outputs map[string]string `yaml:"outputs"` + Result string } // Strategy for the job @@ -433,6 +434,9 @@ func (w *Workflow) GetJob(jobID string) *Job { if j.Name == "" { j.Name = id } + if j.If.Value == "" { + j.If.Value = "success()" + } return j } } diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index 7f9e02da..ac024552 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -4,6 +4,8 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" + "fmt" "io" "os" "path/filepath" @@ -25,6 +27,7 @@ func init() { // NewExpressionEvaluator creates a new evaluator func (rc *RunContext) NewExpressionEvaluator() ExpressionEvaluator { vm := rc.newVM() + return &expressionEvaluator{ vm, } @@ -36,6 +39,10 @@ func (sc *StepContext) NewExpressionEvaluator() ExpressionEvaluator { configers := []func(*otto.Otto){ sc.vmEnv(), sc.vmInputs(), + + sc.vmNeeds(), + sc.vmSuccess(), + sc.vmFailure(), } for _, configer := range configers { configer(vm) @@ -373,14 +380,33 @@ func (rc *RunContext) vmHashFiles() func(*otto.Otto) { func (rc *RunContext) vmSuccess() func(*otto.Otto) { return func(vm *otto.Otto) { _ = vm.Set("success", func() bool { - return rc.getJobContext().Status == "success" + jobs := rc.Run.Workflow.Jobs + jobNeeds := rc.getNeedsTransitive(rc.Run.Job()) + + for _, needs := range jobNeeds { + if jobs[needs].Result != "success" { + return false + } + } + + return true }) } } + func (rc *RunContext) vmFailure() func(*otto.Otto) { return func(vm *otto.Otto) { _ = vm.Set("failure", func() bool { - return rc.getJobContext().Status == "failure" + jobs := rc.Run.Workflow.Jobs + jobNeeds := rc.getNeedsTransitive(rc.Run.Job()) + + for _, needs := range jobNeeds { + if jobs[needs].Result == "failure" { + return true + } + } + + return false }) } } @@ -440,9 +466,9 @@ func (sc *StepContext) vmInputs() func(*otto.Otto) { } } -func (rc *RunContext) vmNeeds() func(*otto.Otto) { - jobs := rc.Run.Workflow.Jobs - jobNeeds := rc.Run.Job().Needs() +func (sc *StepContext) vmNeeds() func(*otto.Otto) { + jobs := sc.RunContext.Run.Workflow.Jobs + jobNeeds := sc.RunContext.Run.Job().Needs() using := make(map[string]map[string]map[string]string) for _, needs := range jobNeeds { @@ -457,6 +483,70 @@ func (rc *RunContext) vmNeeds() func(*otto.Otto) { } } +func (sc *StepContext) vmSuccess() func(*otto.Otto) { + return func(vm *otto.Otto) { + _ = vm.Set("success", func() bool { + return sc.RunContext.getJobContext().Status == "success" + }) + } +} + +func (sc *StepContext) vmFailure() func(*otto.Otto) { + return func(vm *otto.Otto) { + _ = vm.Set("failure", func() bool { + return sc.RunContext.getJobContext().Status == "failure" + }) + } +} + +type vmNeedsStruct struct { + Outputs map[string]string `json:"outputs"` + Result string `json:"result"` +} + +func (rc *RunContext) vmNeeds() func(*otto.Otto) { + return func(vm *otto.Otto) { + needsFunc := func() otto.Value { + jobs := rc.Run.Workflow.Jobs + jobNeeds := rc.Run.Job().Needs() + + using := make(map[string]vmNeedsStruct) + for _, needs := range jobNeeds { + using[needs] = vmNeedsStruct{ + Outputs: jobs[needs].Outputs, + Result: jobs[needs].Result, + } + } + + log.Debugf("context needs => %+v", using) + + value, err := vm.ToValue(using) + if err != nil { + return vm.MakeTypeError(err.Error()) + } + + return value + } + + // Results might change after the Otto VM was created + // and initialized. To access the current state + // we can't just pass a copy to Otto - instead we + // created a 'live-binding'. + // Technical Note: We don't want to pollute the global + // js namespace (and add things github actions hasn't) + // we delete the helper function after installing it + // as a getter. + global, _ := vm.Run("this") + _ = global.Object().Set("__needs__", needsFunc) + _, _ = vm.Run(` + (function (global) { + Object.defineProperty(global, 'needs', { get: global.__needs__ }); + delete global.__needs__; + })(this) + `) + } +} + func (rc *RunContext) vmJob() func(*otto.Otto) { job := rc.getJobContext() @@ -518,3 +608,48 @@ func (rc *RunContext) vmMatrix() func(*otto.Otto) { _ = vm.Set("matrix", rc.Matrix) } } + +// EvalBool evaluates an expression against given evaluator +func EvalBool(evaluator ExpressionEvaluator, expr string) (bool, error) { + if splitPattern == nil { + splitPattern = regexp.MustCompile(fmt.Sprintf(`%s|%s|\S+`, expressionPattern.String(), operatorPattern.String())) + } + if strings.HasPrefix(strings.TrimSpace(expr), "!") { + return false, errors.New("expressions starting with ! must be wrapped in ${{ }}") + } + if expr != "" { + parts := splitPattern.FindAllString(expr, -1) + var evaluatedParts []string + for i, part := range parts { + if operatorPattern.MatchString(part) { + evaluatedParts = append(evaluatedParts, part) + continue + } + + interpolatedPart, isString := evaluator.InterpolateWithStringCheck(part) + + // This peculiar transformation has to be done because the GitHub parser + // treats false returned from contexts as a string, not a boolean. + // Hence env.SOMETHING will be evaluated to true in an if: expression + // regardless if SOMETHING is set to false, true or any other string. + // It also handles some other weirdness that I found by trial and error. + if (expressionPattern.MatchString(part) && // it is an expression + !strings.Contains(part, "!")) && // but it's not negated + interpolatedPart == "false" && // and the interpolated string is false + (isString || previousOrNextPartIsAnOperator(i, parts)) { // and it's of type string or has an logical operator before or after + interpolatedPart = fmt.Sprintf("'%s'", interpolatedPart) // then we have to quote the false expression + } + + evaluatedParts = append(evaluatedParts, interpolatedPart) + } + + joined := strings.Join(evaluatedParts, " ") + v, _, err := evaluator.Evaluate(fmt.Sprintf("Boolean(%s)", joined)) + if err != nil { + return false, err + } + log.Debugf("expression '%s' evaluated to '%s'", expr, v) + return v == "true", nil + } + return true, nil +} diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go old mode 100755 new mode 100644 index e813c0af..cfe37ac3 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -3,7 +3,6 @@ package runner import ( "context" "encoding/json" - "errors" "fmt" "os" "path/filepath" @@ -288,7 +287,20 @@ func (rc *RunContext) Executor() common.Executor { } steps = append(steps, rc.newStepExecutor(step)) } - steps = append(steps, rc.stopJobContainer()) + steps = append(steps, func(ctx context.Context) error { + err := rc.stopJobContainer()(ctx) + if err != nil { + return err + } + + rc.Run.Job().Result = "success" + jobError := common.JobError(ctx) + if jobError != nil { + rc.Run.Job().Result = "failure" + } + + return nil + }) return common.NewPipelineExecutor(steps...).Finally(rc.interpolateOutputs()).Finally(func(ctx context.Context) error { if rc.JobContainer != nil { @@ -310,15 +322,9 @@ func (rc *RunContext) newStepExecutor(step *model.Step) common.Executor { Conclusion: stepStatusSuccess, Outputs: make(map[string]string), } - runStep, err := rc.EvalBool(sc.Step.If.Value) + runStep, err := sc.isEnabled(ctx) if err != nil { - common.Logger(ctx).Errorf(" \u274C Error in if: expression - %s", sc.Step) - exprEval, err := sc.setupEnv(ctx) - if err != nil { - return err - } - rc.ExprEval = exprEval rc.StepResults[rc.CurrentStep].Conclusion = stepStatusFailure rc.StepResults[rc.CurrentStep].Outcome = stepStatusFailure return err @@ -403,7 +409,7 @@ func (rc *RunContext) hostname() string { func (rc *RunContext) isEnabled(ctx context.Context) bool { job := rc.Run.Job() l := common.Logger(ctx) - runJob, err := rc.EvalBool(job.If.Value) + runJob, err := EvalBool(rc.ExprEval, job.If.Value) if err != nil { common.Logger(ctx).Errorf(" \u274C Error in if: expression - %s", job.Name) return false @@ -430,51 +436,6 @@ func (rc *RunContext) isEnabled(ctx context.Context) bool { var splitPattern *regexp.Regexp -// EvalBool evaluates an expression against current run context -func (rc *RunContext) EvalBool(expr string) (bool, error) { - if splitPattern == nil { - splitPattern = regexp.MustCompile(fmt.Sprintf(`%s|%s|\S+`, expressionPattern.String(), operatorPattern.String())) - } - if strings.HasPrefix(strings.TrimSpace(expr), "!") { - return false, errors.New("expressions starting with ! must be wrapped in ${{ }}") - } - if expr != "" { - parts := splitPattern.FindAllString(expr, -1) - var evaluatedParts []string - for i, part := range parts { - if operatorPattern.MatchString(part) { - evaluatedParts = append(evaluatedParts, part) - continue - } - - interpolatedPart, isString := rc.ExprEval.InterpolateWithStringCheck(part) - - // This peculiar transformation has to be done because the GitHub parser - // treats false returned from contexts as a string, not a boolean. - // Hence env.SOMETHING will be evaluated to true in an if: expression - // regardless if SOMETHING is set to false, true or any other string. - // It also handles some other weirdness that I found by trial and error. - if (expressionPattern.MatchString(part) && // it is an expression - !strings.Contains(part, "!")) && // but it's not negated - interpolatedPart == "false" && // and the interpolated string is false - (isString || previousOrNextPartIsAnOperator(i, parts)) { // and it's of type string or has an logical operator before or after - interpolatedPart = fmt.Sprintf("'%s'", interpolatedPart) // then we have to quote the false expression - } - - evaluatedParts = append(evaluatedParts, interpolatedPart) - } - - joined := strings.Join(evaluatedParts, " ") - v, _, err := rc.ExprEval.Evaluate(fmt.Sprintf("Boolean(%s)", joined)) - if err != nil { - return false, err - } - log.Debugf("expression '%s' evaluated to '%s'", expr, v) - return v == "true", nil - } - return true, nil -} - func previousOrNextPartIsAnOperator(i int, parts []string) bool { operator := false if i > 0 { @@ -557,6 +518,17 @@ func (rc *RunContext) getStepsContext() map[string]*stepResult { return rc.StepResults } +func (rc *RunContext) getNeedsTransitive(job *model.Job) []string { + needs := job.Needs() + + for _, need := range needs { + parentNeeds := rc.getNeedsTransitive(rc.Run.Workflow.GetJob(need)) + needs = append(needs, parentNeeds...) + } + + return needs +} + type githubContext struct { Event map[string]interface{} `json:"event"` EventPath string `json:"event_path"` diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index 0419f4a3..610b0370 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -1,6 +1,7 @@ package runner import ( + "context" "fmt" "os" "regexp" @@ -153,7 +154,7 @@ func TestRunContext_EvalBool(t *testing.T) { t.Run(table.in, func(t *testing.T) { assertObject := assert.New(t) defer hook.Reset() - b, err := rc.EvalBool(table.in) + b, err := EvalBool(rc.ExprEval, table.in) if table.wantErr { assertObject.Error(err) } @@ -178,7 +179,7 @@ func updateTestIfWorkflow(t *testing.T, tables []struct { for _, k := range keys { envs += fmt.Sprintf(" %s: %s\n", k, rc.Env[k]) } - + // editorconfig-checker-disable workflow := fmt.Sprintf(` name: "Test what expressions result in true and false on GitHub" on: push @@ -191,6 +192,7 @@ jobs: runs-on: ubuntu-latest steps: `, envs) + // editorconfig-checker-enable for i, table := range tables { if table.wantErr || strings.HasPrefix(table.in, "github.actor") { @@ -344,3 +346,138 @@ func TestGetGitHubContext(t *testing.T) { assert.Equal(t, ghc.EventPath, ActPath+"/workflow/event.json") assert.Equal(t, ghc.Token, rc.Config.Secrets["GITHUB_TOKEN"]) } + +func createIfTestRunContext(jobs map[string]*model.Job) *RunContext { + rc := &RunContext{ + Config: &Config{ + Workdir: ".", + Platforms: map[string]string{ + "ubuntu-latest": "ubuntu-latest", + }, + }, + Env: map[string]string{}, + Run: &model.Run{ + JobID: "job1", + Workflow: &model.Workflow{ + Name: "test-workflow", + Jobs: jobs, + }, + }, + } + rc.ExprEval = rc.NewExpressionEvaluator() + + return rc +} + +func createJob(t *testing.T, input string, result string) *model.Job { + var job *model.Job + err := yaml.Unmarshal([]byte(input), &job) + assert.NoError(t, err) + job.Result = result + + return job +} + +func TestRunContextIsEnabled(t *testing.T) { + log.SetLevel(log.DebugLevel) + assertObject := assert.New(t) + + // success() + rc := createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest +if: success()`, ""), + }) + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "failure"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: success()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.False(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "success"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: success()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "failure"), + "job2": createJob(t, `runs-on: ubuntu-latest +if: success()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) + + // failure() + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest +if: failure()`, ""), + }) + assertObject.False(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "failure"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: failure()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "success"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: failure()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.False(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "failure"), + "job2": createJob(t, `runs-on: ubuntu-latest +if: failure()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.False(rc.isEnabled(context.Background())) + + // always() + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest +if: always()`, ""), + }) + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "failure"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: always()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "success"), + "job2": createJob(t, `runs-on: ubuntu-latest +needs: [job1] +if: always()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) + + rc = createIfTestRunContext(map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, "success"), + "job2": createJob(t, `runs-on: ubuntu-latest +if: always()`, ""), + }) + rc.Run.JobID = "job2" + assertObject.True(rc.isEnabled(context.Background())) +} diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 5fb88483..34fab71d 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -154,7 +154,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { } return nil - })(WithJobLogger(ctx, jobName, rc.Config.Secrets, rc.Config.InsecureSecrets)) + })(common.WithJobErrorContainer(WithJobLogger(ctx, jobName, rc.Config.Secrets, rc.Config.InsecureSecrets))) }) b++ if b == maxParallel { @@ -166,7 +166,20 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { } } - return common.NewPipelineExecutor(pipeline...) + return common.NewPipelineExecutor(pipeline...).Then(handleFailure(plan)) +} + +func handleFailure(plan *model.Plan) common.Executor { + return func(ctx context.Context) error { + for _, stage := range plan.Stages { + for _, run := range stage.Runs { + if run.Job().Result == "failure" { + return fmt.Errorf("Job '%s' failed", run.String()) + } + } + } + return nil + } } func (runner *runnerImpl) newRunContext(run *model.Run, matrix map[string]interface{}) *RunContext { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index bffa0d67..7ca000c3 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -123,6 +123,8 @@ func TestRunEvent(t *testing.T) { {"testdata", "outputs", "push", "", platforms, ""}, {"testdata", "steps-context/conclusion", "push", "", platforms, ""}, {"testdata", "steps-context/outcome", "push", "", platforms, ""}, + {"testdata", "job-status-check", "push", "job 'fail' failed", platforms, ""}, + {"testdata", "if-expressions", "push", "Job 'mytest' failed", platforms, ""}, {"../model/testdata", "strategy", "push", "", platforms, ""}, // TODO: move all testdata into pkg so we can validate it with planner and runner // {"testdata", "issue-228", "push", "", platforms, ""}, // TODO [igni]: Remove this once everything passes diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index 367d7fe8..e69aa402 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -145,6 +145,21 @@ func (sc *StepContext) interpolateEnv(exprEval ExpressionEvaluator) { } } +func (sc *StepContext) isEnabled(ctx context.Context) (bool, error) { + runStep, err := EvalBool(sc.NewExpressionEvaluator(), sc.Step.If.Value) + if err != nil { + common.Logger(ctx).Errorf(" \u274C Error in if: expression - %s", sc.Step) + exprEval, err := sc.setupEnv(ctx) + if err != nil { + return false, err + } + sc.RunContext.ExprEval = exprEval + return false, err + } + + return runStep, nil +} + func (sc *StepContext) setupEnv(ctx context.Context) (ExpressionEvaluator, error) { rc := sc.RunContext sc.Env = sc.mergeEnv() diff --git a/pkg/runner/step_context_test.go b/pkg/runner/step_context_test.go index 7ed544a6..7f5d32a5 100644 --- a/pkg/runner/step_context_test.go +++ b/pkg/runner/step_context_test.go @@ -4,7 +4,12 @@ import ( "context" "testing" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" + "github.com/nektos/act/pkg/common" + "github.com/nektos/act/pkg/model" ) func TestStepContextExecutor(t *testing.T) { @@ -27,3 +32,85 @@ func TestStepContextExecutor(t *testing.T) { runTestJobFile(ctx, t, table) } } + +func createIfTestStepContext(t *testing.T, input string) *StepContext { + var step *model.Step + err := yaml.Unmarshal([]byte(input), &step) + assert.NoError(t, err) + + return &StepContext{ + RunContext: &RunContext{ + Config: &Config{ + Workdir: ".", + Platforms: map[string]string{ + "ubuntu-latest": "ubuntu-latest", + }, + }, + StepResults: map[string]*stepResult{}, + Env: map[string]string{}, + Run: &model.Run{ + JobID: "job1", + Workflow: &model.Workflow{ + Name: "workflow1", + Jobs: map[string]*model.Job{ + "job1": createJob(t, `runs-on: ubuntu-latest`, ""), + }, + }, + }, + }, + Step: step, + } +} + +func TestStepContextIsEnabled(t *testing.T) { + log.SetLevel(log.DebugLevel) + assertObject := assert.New(t) + + // success() + sc := createIfTestStepContext(t, "if: success()") + assertObject.True(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: success()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusSuccess, + } + assertObject.True(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: success()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusFailure, + } + assertObject.False(sc.isEnabled(context.Background())) + + // failure() + sc = createIfTestStepContext(t, "if: failure()") + assertObject.False(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: failure()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusSuccess, + } + assertObject.False(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: failure()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusFailure, + } + assertObject.True(sc.isEnabled(context.Background())) + + // always() + sc = createIfTestStepContext(t, "if: always()") + assertObject.True(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: always()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusSuccess, + } + assertObject.True(sc.isEnabled(context.Background())) + + sc = createIfTestStepContext(t, "if: always()") + sc.RunContext.StepResults["a"] = &stepResult{ + Conclusion: stepStatusFailure, + } + assertObject.True(sc.isEnabled(context.Background())) +} diff --git a/pkg/runner/testdata/if-expressions/push.yml b/pkg/runner/testdata/if-expressions/push.yml new file mode 100644 index 00000000..36535b3d --- /dev/null +++ b/pkg/runner/testdata/if-expressions/push.yml @@ -0,0 +1,29 @@ +on: push +jobs: + mytest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + # - run: exit 1 + - uses: ./ + if: failure() + - run: echo Success + shell: bash + - run: echo Success + if: success() + shell: bash + - run: exit 1 + shell: bash + - run: echo "Shouldn't run" + if: success() + shell: bash + - run: echo "Shouldn't run2" + shell: bash + - run: echo expected to run + if: failure() + shell: bash + next: + needs: mytest + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 diff --git a/pkg/runner/testdata/job-status-check/push.yml b/pkg/runner/testdata/job-status-check/push.yml new file mode 100644 index 00000000..4ae6af86 --- /dev/null +++ b/pkg/runner/testdata/job-status-check/push.yml @@ -0,0 +1,28 @@ +on: push +jobs: + fail: + runs-on: ubuntu-latest + steps: + - run: exit 1 + suc1: + if: success() || failure() + needs: + - fail + runs-on: ubuntu-latest + steps: + - run: exit 0 + suc2: + if: success() || failure() + needs: + - fail + runs-on: ubuntu-latest + steps: + - run: exit 0 + next: + needs: + - suc1 + - suc2 + runs-on: ubuntu-latest + steps: + - run: echo should never reach here + - run: exit 1