From 0bb9c3f1272a83a37b7f9f220c4077c33675b16e Mon Sep 17 00:00:00 2001 From: Marc Cataford Date: Mon, 19 Aug 2024 23:35:33 -0400 Subject: [PATCH] feat: add continue-on-error job-level support --- WORKFLOW_SUPPORT.md | 2 +- internal/commands/execute_workflow.go | 10 +-- internal/runner/mock_container_driver.go | 31 ++++++++-- internal/runner/runner.go | 16 +++-- internal/runner/runner_job_test.go | 77 ++++++++++++++++++++++++ internal/runner/task.go | 14 +++++ internal/runner/task_test.go | 27 +++++++++ internal/workflow/models.go | 6 +- 8 files changed, 167 insertions(+), 16 deletions(-) diff --git a/WORKFLOW_SUPPORT.md b/WORKFLOW_SUPPORT.md index 68c97af..60fd03d 100644 --- a/WORKFLOW_SUPPORT.md +++ b/WORKFLOW_SUPPORT.md @@ -27,7 +27,7 @@ syntax](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for - [ ] jobs..timeout-minutes - [ ] jobs..strategy - [ ] jobs..container - - [ ] jobs..continue-on-error + - [x] jobs..continue-on-error - [ ] jobs..services - [ ] jobs..uses - [ ] jobs..with diff --git a/internal/commands/execute_workflow.go b/internal/commands/execute_workflow.go index 9e9f0eb..311e5b7 100644 --- a/internal/commands/execute_workflow.go +++ b/internal/commands/execute_workflow.go @@ -38,10 +38,6 @@ func ExecuteWorkflow(configuration Configuration, workflowFile string) error { } taskResult := runnerInstance.RunWorkflow(*workflow) - if !taskResult.HasError() { - return nil - } - for _, job := range taskResult.Children { if job.Status == "success" { logger.Info(logger.Green("Job %s: %s"), job.TaskId, job.Status) @@ -50,5 +46,9 @@ func ExecuteWorkflow(configuration Configuration, workflowFile string) error { } } - return fmt.Errorf("Task %s failed with at least 1 error.", taskResult.TaskId) + if taskResult.Failed() { + return fmt.Errorf("Task %s failed with at least 1 error.", taskResult.TaskId) + } + + return nil } diff --git a/internal/runner/mock_container_driver.go b/internal/runner/mock_container_driver.go index aeb6133..8616517 100644 --- a/internal/runner/mock_container_driver.go +++ b/internal/runner/mock_container_driver.go @@ -1,6 +1,7 @@ package runner import ( + "fmt" "strings" ) @@ -52,10 +53,19 @@ func (d *MockDriver) Exec(containerName string, command string, cwd string) Comm if _, init := d.calls["Exec"]; !init { d.calls["Exec"] = []MockCall{} } - d.calls["Exec"] = append(d.calls["Exec"], MockCall{fname: "Exec", args: []string{containerName, command, cwd}}) - if _, mocked := d.mockedCalls["Exec"][strings.Join([]string{containerName, command, cwd}, " ")]; mocked { - return d.mockedCalls["Exec"][strings.Join([]string{containerName, command, cwd}, " ")] + args := []string{containerName, command, cwd} + d.calls["Exec"] = append(d.calls["Exec"], MockCall{fname: "Exec", args: args}) + + mockKeys := []string{ + fmt.Sprintf("nthcall::%d", len(d.calls["Exec"])), + fmt.Sprintf("withargs::%s", strings.Join(args, " ")), + } + + for _, mockKey := range mockKeys { + if _, mocked := d.mockedCalls["Exec"][mockKey]; mocked { + return d.mockedCalls["Exec"][mockKey] + } } return CommandResult{} @@ -65,7 +75,20 @@ func (d *MockDriver) Exec(containerName string, command string, cwd string) Comm // // The mocked call is reused as long as it's defined within the mock driver. func (d *MockDriver) WithMockedCall(fn string, returnValue CommandResult, args ...string) { - mockKey := strings.Join(args, " ") + mockKey := fmt.Sprintf("withargs::%s", strings.Join(args, " ")) + + if _, initialized := d.mockedCalls[fn]; !initialized { + d.mockedCalls[fn] = map[string]CommandResult{} + } + + d.mockedCalls[fn][mockKey] = returnValue +} + +// Mocks the nth call to to return . +// +// The mocked call is reused as long as it's defined within the mock driver. +func (d *MockDriver) WithNthMockedCall(fn string, callIndex int, returnValue CommandResult) { + mockKey := fmt.Sprintf("nthcall::%d", callIndex) if _, initialized := d.mockedCalls[fn]; !initialized { d.mockedCalls[fn] = map[string]CommandResult{} diff --git a/internal/runner/runner.go b/internal/runner/runner.go index da2b987..b7c81f9 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -64,13 +64,21 @@ func (r *Runner) RunWorkflow(workflow workflow.Workflow) TaskTracker { logger.Info("Using image %s (label: %s)", runnerImage, job.RunsOn) if pullError := runner.Driver.Pull(runnerImage); pullError != nil { - jobTracker.SetStatus("failed").SetError(pullError) - return + jobTracker.SetError(pullError) + + if !job.ContinueOnError { + jobTracker.SetStatus("failed") + return + } + } if runError := runner.RunJobInContainer(runnerImage, containerName, jobContext); runError != nil { - jobTracker.SetStatus("failed").SetError(runError) - return + jobTracker.SetError(runError) + if !job.ContinueOnError { + jobTracker.SetStatus("failed") + return + } } jobTracker.SetStatus("success") diff --git a/internal/runner/runner_job_test.go b/internal/runner/runner_job_test.go index 1c376e5..77edf51 100644 --- a/internal/runner/runner_job_test.go +++ b/internal/runner/runner_job_test.go @@ -7,6 +7,83 @@ import ( "testing" ) +func TestJobErrorsWorkflowErrorsIfJobContinueOnErrorUndefined(t *testing.T) { + workflowSample := ` +jobs: + jobA: + runs-on: test + steps: + - run: exit 1 + ` + + workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) + mockDriver := NewMockDriver() + + runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) + + jobContext := context.WithValue(context.Background(), "workflow", *workflow) + jobContext = context.WithValue(jobContext, "currentJob", workflow.Jobs["jobA"]) + + mockDriver.WithNthMockedCall("Exec", 1, CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}) + task := runner.RunWorkflow(*workflow) + + if !task.Failed() { + t.Error("Expected task to fail, but it succeeded.") + } +} +func TestJobErrorsWorkflowErrorsIfJobContinueOnErrorFalsy(t *testing.T) { + workflowSample := ` +jobs: + jobA: + runs-on: test + continue-on-error: false + steps: + - run: exit 1 + ` + + workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) + mockDriver := NewMockDriver() + + runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) + + jobContext := context.WithValue(context.Background(), "workflow", *workflow) + jobContext = context.WithValue(jobContext, "currentJob", workflow.Jobs["jobA"]) + + mockDriver.WithNthMockedCall("Exec", 1, CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}) + task := runner.RunWorkflow(*workflow) + + if !task.Failed() { + t.Error("Expected task to fail, but it succeeded.") + } +} + +func TestJobSkipsErrorWorkflowSucceedsIfContinueOnErrorTruthy(t *testing.T) { + workflowSample := ` +jobs: + jobA: + runs-on: test + continue-on-error: true + steps: + - run: exit 1 + ` + + workflow, _ := workflow.FromYamlBytes([]byte(workflowSample)) + mockDriver := NewMockDriver() + + mockDriver.WithMockedCall("Exec", CommandResult{Error: errors.New("exit 1!"), ExitCode: 1}, "testContainer", "exit 1") + runner := NewRunner(&mockDriver, map[string]string{"test": "test"}) + + jobContext := context.WithValue(context.Background(), "workflow", *workflow) + jobContext = context.WithValue(jobContext, "currentJob", workflow.Jobs["jobA"]) + + task := runner.RunWorkflow(*workflow) + + if task.Failed() { + t.Error("Expected task to succeed, but it failed.") + } + +} + func TestJobStepSkipsErrorIfContinueOnErrorTruthy(t *testing.T) { workflowSample := ` jobs: diff --git a/internal/runner/task.go b/internal/runner/task.go index e97a6f2..bf5d1f8 100644 --- a/internal/runner/task.go +++ b/internal/runner/task.go @@ -52,3 +52,17 @@ func (t TaskTracker) HasError() bool { return false } + +func (t TaskTracker) Failed() bool { + if t.Status == "failed" { + return true + } + + for _, child := range t.Children { + if child.Failed() { + return true + } + } + + return false +} diff --git a/internal/runner/task_test.go b/internal/runner/task_test.go index 53351ec..be0c3e5 100644 --- a/internal/runner/task_test.go +++ b/internal/runner/task_test.go @@ -27,3 +27,30 @@ func TestTaskHasErrorReturnsTrueIfAnyJobHasErrors(t *testing.T) { t.Errorf("Expected true, got false.") } } + +func TestTaskFailedReturnsTrueIfAnyChildHasFailed(t *testing.T) { + task := NewTaskTracker(nil) + NewTaskTracker(task).SetStatus("failed") + + if !task.Failed() { + t.Error("Expected failed = true, got false.") + } +} + +func TestTaskFailedReturnsTrueIfTaskFailed(t *testing.T) { + task := NewTaskTracker(nil).SetStatus("failed") + NewTaskTracker(task) + + if !task.Failed() { + t.Error("Expected failed = true, got false.") + } +} + +func TeskTaskFailedReturnsFalseIfNeitherSelfOrChildrenFailed(t *testing.T) { + task := NewTaskTracker(nil).SetStatus("success") + NewTaskTracker(task) + + if task.Failed() { + t.Error("Expected failed = false, got true.") + } +} diff --git a/internal/workflow/models.go b/internal/workflow/models.go index 133deef..fefacd4 100644 --- a/internal/workflow/models.go +++ b/internal/workflow/models.go @@ -9,8 +9,10 @@ type Job struct { Steps []Step `yaml:"steps"` Needs []string `yaml:"needs"` // Job name; this isn't guaranteed to be unique. - Name string `yaml:"name"` - Defaults struct { + Name string `yaml:"name"` + // If truthy, job-level failure does not bubble up to the workflow. + ContinueOnError bool `yaml:"continue-on-error"` + Defaults struct { Run struct { WorkingDirectory string `yaml:"working-directory"` } `yaml:"run"`