diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 4771f46..dd7d79f 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -183,11 +183,13 @@ Map-form entries accept three optional keys. String-form entries do not: the shape selects the capabilities, so an entry needing an override is written as a map. -| Key | Type | Effect | -| -------- | ------ | ----------------------------------------------------------------------------- | -| `env` | map | Layers over the group's `env:` for this command only. | -| `dir` | string | Runs this command in another directory, without needing a shell. | -| `silent` | bool | Suppresses live streaming; the output is still captured. | +| Key | Type | Effect | +| ------------------- | ------ | ----------------------------------------------------------------- | +| `env` | map | Layers over the group's `env:` for this command only. | +| `dir` | string | Runs this command in another directory, without needing a shell. | +| `silent` | bool | Suppresses live streaming; the output is still captured. | +| `continue-on-error` | bool | This command's failure is tolerated; the sequence continues. | +| `always` | bool | Runs even when an earlier command already failed. | ```yaml groups: @@ -223,11 +225,50 @@ commands: SHA: '{{ output "build" }}' ``` -`env` and `dir` fold into the cache fingerprint, so changing either re-runs the -group. `silent` does not: it changes nothing about what the command does or -produces, so toggling it keeps a valid cached result. A group that uses none of -these keys keeps the fingerprint it had before they existed — upgrading does not -invalidate existing caches. +### Failure policy + +By default a sequence stops at the first non-zero exit, like `set -e`. Two +independent keys bend that, and they compose: + +```yaml +groups: + - name: test-with-teardown + commands: + - ./setup.sh # starts a database + - { command: ./optional-lint.sh, continue-on-error: true } + - { command: go, params: [test, "./..."] } + - { command: ./teardown.sh, always: true } # runs even if tests fail +``` + +`continue-on-error` says this command's outcome does not matter: it is logged as +a warning, the sequence continues, and the group still succeeds with `exitCode` +0. Use it for best-effort steps. + +`always` guarantees the command runs even after an earlier one failed. It does +**not** forgive that failure — the group still fails, reporting the original +error. This is how teardown works. When an `always` command fails after an +earlier failure, the first error is reported and the second is logged, so the +diagnostic keeps pointing at the real cause. Combine both keys when a cleanup +step must run and its own failure should also be ignored. + +Entries between the failure and an `always` entry are skipped, not run. + +`always` also survives cancellation: when the group hits its `timeout:` or you +interrupt the run, pending `always` commands still execute, detached from the +dead deadline and bounded by a 30-second grace period. A hung test that trips a +timeout still gets its teardown. Their own failures are logged rather than +reported, since the cancellation is the real cause. + +One limit worth knowing: **`retries:` replays the whole sequence**, so an +`always` teardown runs once per attempt. Keep it idempotent. + +### Caching interaction + +`env`, `dir`, `continue-on-error`, and `always` all fold into the cache +fingerprint, so changing any of them re-runs the group. `silent` does not: it +changes nothing about what the command does or produces, so toggling it keeps a +valid cached result. A group that uses none of these keys keeps the fingerprint +it had before they existed — upgrading does not invalidate existing caches. ### Referencing other groups' output diff --git a/internal/cache/cache.go b/internal/cache/cache.go index 860790a..9664704 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -37,8 +37,8 @@ type Store interface { } // Compute returns a content fingerprint for the given cache spec. The -// fingerprint changes when the method, any command/param/form/env/dir in the -// group's command list, or any matched input file changes. For shell-form entries the +// fingerprint changes when the method, anything about a command in the group's +// list except silent, or any matched input file changes. For shell-form entries the // fingerprint also changes when the shell program changes. A glob that matches // nothing contributes nothing, so adding the first matching file naturally // changes the fingerprint. @@ -67,6 +67,14 @@ func Compute(spec *config.Cache, shell string, commands []config.CommandSpec) (s for _, k := range slices.Sorted(maps.Keys(c.Env)) { fmt.Fprintf(h, "%s\x04%s\x03", k, c.Env[k]) } + // Distinct markers: the two policies change whether the group + // succeeds, so they must never hash alike. + if c.ContinueOnError { + fmt.Fprintf(h, "coe\x03") + } + if c.Always { + fmt.Fprintf(h, "alw\x03") + } fmt.Fprintf(h, "\x02") } diff --git a/internal/cache/percommand_test.go b/internal/cache/percommand_test.go index c9c017c..b9735f2 100644 --- a/internal/cache/percommand_test.go +++ b/internal/cache/percommand_test.go @@ -47,6 +47,21 @@ func TestCompute_PerCommandKnobs(t *testing.T) { spec: config.CommandSpec{Command: "go", Params: []string{"build"}, Silent: true}, wantBust: false, }, + { + name: "continue-on-error busts", + spec: config.CommandSpec{Command: "go", Params: []string{"build"}, ContinueOnError: true}, + wantBust: true, + }, + { + name: "always busts", + spec: config.CommandSpec{Command: "go", Params: []string{"build"}, Always: true}, + wantBust: true, + }, + { + name: "the two policy keys are distinguishable", + spec: config.CommandSpec{Command: "go", Params: []string{"build"}, Always: true, ContinueOnError: true}, + wantBust: true, + }, { name: "empty env map is the same as no env", spec: config.CommandSpec{Command: "go", Params: []string{"build"}, Env: map[string]string{}}, @@ -82,6 +97,14 @@ func TestCompute_EnvOrderIsDeterministic(t *testing.T) { } } +func TestCompute_PolicyKeysAreDistinct(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "a.go"), "package main\n") + soft := config.CommandSpec{Command: "go", ContinueOnError: true} + always := config.CommandSpec{Command: "go", Always: true} + assert.NotEqual(t, fpFor(t, dir, soft), fpFor(t, dir, always)) +} + // Two different env maps must not hash the same just because their // concatenated bytes could line up. func TestCompute_EnvPairsAreUnambiguous(t *testing.T) { diff --git a/internal/config/commands_test.go b/internal/config/commands_test.go index d6ae8b6..ab525e4 100644 --- a/internal/config/commands_test.go +++ b/internal/config/commands_test.go @@ -106,6 +106,31 @@ func TestCommandSpec_UnmarshalYAML_PerCommandKnobs(t *testing.T) { Env: map[string]string{"A": "1", "B": "2"}, }, }, + { + name: "per-command continue-on-error", + yaml: `{command: ./lint.sh, continue-on-error: true}`, + want: CommandSpec{Command: "./lint.sh", ContinueOnError: true}, + }, + { + name: "per-command always", + yaml: `{command: ./teardown.sh, always: true}`, + want: CommandSpec{Command: "./teardown.sh", Always: true}, + }, + { + name: "failure policy keys compose", + yaml: `{command: ./cleanup.sh, always: true, continue-on-error: true}`, + want: CommandSpec{Command: "./cleanup.sh", Always: true, ContinueOnError: true}, + }, + { + name: "continue-on-error must be a boolean", + yaml: `{command: go, continue-on-error: sometimes}`, + wantErr: `"continue-on-error" must be a boolean`, + }, + { + name: "always must be a boolean", + yaml: `{command: go, always: yep}`, + wantErr: `"always" must be a boolean`, + }, { name: "empty dir rejected", yaml: `{command: go, dir: ""}`, @@ -514,3 +539,23 @@ func TestLoadConfig_PerCommandKnobsFixture(t *testing.T) { require.NoError(t, err) assert.Contains(t, refs, "single", "an env template must register as a dependency") } + +func TestLoadConfig_FailurePolicyFixture(t *testing.T) { + cfg, err := LoadConfig("./test-resources/config-commands-valid.yml") + require.NoError(t, err) + + policy := cfg.GroupByName("policy") + require.NotNil(t, policy) + list := policy.CommandList() + require.Len(t, list, 4) + + assert.True(t, list[0].ContinueOnError) + assert.False(t, list[0].Always) + assert.True(t, list[2].Always) + assert.False(t, list[2].ContinueOnError) + assert.True(t, list[3].Always, "the two keys compose on one entry") + assert.True(t, list[3].ContinueOnError) + + assert.False(t, list[1].Always, "an entry declaring neither stays strict") + assert.False(t, list[1].ContinueOnError) +} diff --git a/internal/config/config.go b/internal/config/config.go index d5702a6..4b57705 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -220,6 +220,10 @@ type CommandSpec struct { Env map[string]string `yaml:"env,omitempty" json:"env,omitempty"` Dir string `yaml:"dir,omitempty" json:"dir,omitempty"` Silent bool `yaml:"silent,omitempty" json:"silent,omitempty"` + // Independent and composable: Always guarantees execution, ContinueOnError + // forgives the outcome. + ContinueOnError bool `yaml:"continue-on-error,omitempty" json:"continueOnError,omitempty"` + Always bool `yaml:"always,omitempty" json:"always,omitempty"` // IsShell records that the entry was written in string form and therefore // runs through the group's shell:. Argv-form entries always exec directly // and ignore shell:, even when it is set. @@ -288,15 +292,24 @@ func (cs *CommandSpec) unmarshalKey(key string, val *yaml.Node) error { } cs.Dir = val.Value case "silent": - if err := val.Decode(&cs.Silent); err != nil { - return fmt.Errorf(`commands entry: "silent" must be a boolean: %w`, err) - } + return decodeBool(val, key, &cs.Silent) + case "continue-on-error": + return decodeBool(val, key, &cs.ContinueOnError) + case "always": + return decodeBool(val, key, &cs.Always) default: return fmt.Errorf("commands entry: unexpected key %q (use a string or a {command, params} map)", key) } return nil } +func decodeBool(val *yaml.Node, key string, dst *bool) error { + if err := val.Decode(dst); err != nil { + return fmt.Errorf("commands entry: %q must be a boolean: %w", key, err) + } + return nil +} + // NewConfig parses YAML bytes into a Config and validates the schema. func NewConfig(b []byte) (*Config, error) { var cfg Config diff --git a/internal/config/test-resources/config-commands-valid.yml b/internal/config/test-resources/config-commands-valid.yml index 386bf53..a9ca17f 100644 --- a/internal/config/test-resources/config-commands-valid.yml +++ b/internal/config/test-resources/config-commands-valid.yml @@ -37,6 +37,15 @@ groups: env: SHA: '{{ output "single" }}' + # failure policy — a tolerated failure, then a teardown that always runs + - name: policy + description: "Per-command failure policy" + commands: + - { command: "false", continue-on-error: true } + - { command: echo, params: [work] } + - { command: echo, params: [teardown], always: true } + - { command: "false", always: true, continue-on-error: true } + flows: ci: description: "Multi then single" @@ -44,5 +53,6 @@ flows: - run: [multi] - run: [single] - run: [knobs] + - run: [policy] default: ci diff --git a/internal/engine/commands_test.go b/internal/engine/commands_test.go index 107d201..2a0180a 100644 --- a/internal/engine/commands_test.go +++ b/internal/engine/commands_test.go @@ -245,6 +245,14 @@ func TestEngine_CommandsFixture_EndToEnd(t *testing.T) { single, ok := e.Outputs().Get("single") require.True(t, ok) assert.Equal(t, "single-step\n", single.Output) + + // Two entries exit non-zero; both are tolerated, so the group succeeds and + // publishes only the surviving commands' output. + policy, ok := e.Outputs().Get("policy") + require.True(t, ok) + assert.Equal(t, "work\nteardown\n", policy.Output) + assert.Equal(t, 0, policy.ExitCode) + assert.Equal(t, result.StatusOK, policy.Status) } func TestEngine_ErrorDecoration_SingularVsMulti(t *testing.T) { diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 45bccb0..8d42828 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -339,30 +339,42 @@ func (e *Engine) execWithEnvelope( } // runSequence executes the group's expanded commands in declared order, -// stopping at the first failure (like set -e). The returned RunResult -// aggregates the whole sequence: concatenated streams, summed duration, the -// exit code of the first failing command (0 when all succeed), and the last -// runner-reported status. Each command goes to the runner as a self-contained -// copy of the group with exactly one command set; argv-form entries clear -// Shell so they always safe-exec, string-form entries keep the group's shell. +// stopping at the first failure (like set -e). Two per-entry knobs bend that: +// always runs an entry even after an earlier failure, and continue-on-error +// tolerates an entry's own failure. The returned RunResult aggregates the whole +// sequence: concatenated streams, summed duration, the exit code of the first +// failing command (0 when all succeed), and the last runner-reported status. +// Each command goes to the runner as a self-contained copy of the group with +// exactly one command set; argv-form entries clear Shell so they always +// safe-exec, string-form entries keep the group's shell. func (e *Engine) runSequence(ctx context.Context, group *config.Group, commands []config.CommandSpec) (result.RunResult, error) { var agg result.RunResult + var firstErr error for i, s := range commands { if err := ctx.Err(); err != nil { - return agg, err + e.runDetachedAlways(ctx, group, commands[i:], &agg) + if firstErr == nil { + firstErr = err + } + return agg, firstErr + } + if firstErr != nil && !s.Always { + continue } - sg := *group - sg.Command = s.Command - sg.Params = s.Params - sg.Commands = nil // the copy presents exactly one command - if !s.IsShell { - sg.Shell = "" // {command, params} entries are always safe argv exec + out, err := e.runner.Run(ctx, e.commandGroup(group, s), s, e.cfg.Env) + aggregate(&agg, &out) + + if err != nil && s.ContinueOnError { + // The exit code must not leak into a result reported as + // successful, but Status still needs a value: "" is reserved for + // groups that never ran. + if agg.Status == "" { + agg.Status = result.StatusOK + } + e.log.Warn("command failed; continuing", + "group", group.Name, "command", s.Command, "err", err.Error()) + continue } - out, err := e.runner.Run(ctx, &sg, s, e.cfg.Env) - agg.Stdout += out.Stdout - agg.Stderr += out.Stderr - agg.Output += out.Output - agg.DurationMs += out.DurationMs // First non-ok status wins (mirroring ExitCode): a soft-fail Runner's // Status:"failed" must survive later successful commands, per the // trust-the-runner contract in runGroup. @@ -372,16 +384,75 @@ func (e *Engine) runSequence(ctx context.Context, group *config.Group, commands if agg.ExitCode == 0 { agg.ExitCode = out.ExitCode } + if err == nil { + continue + } + // Keep singular-group error strings identical to the pre-multi + // behavior; only decorate when there is a sequence to point into. + if len(commands) > 1 { + err = fmt.Errorf("command %d of %d: %w", i+1, len(commands), err) + } + if firstErr == nil { + firstErr = err + continue + } + e.log.Warn("always command failed after an earlier failure", + "group", group.Name, "command", s.Command, "err", err.Error()) + } + return agg, firstErr +} + +// teardownGrace bounds always entries once the run's own context is dead, so a +// hung teardown cannot outlive the run indefinitely. +const teardownGrace = 30 * time.Second + +// runDetachedAlways runs the always entries still pending when the context was +// canceled. A timeout or interrupt is precisely when teardown matters, so they +// run under a fresh deadline rather than the dead one; their failures are +// logged, never returned, because the cancellation is the reported cause. +func (e *Engine) runDetachedAlways( + ctx context.Context, group *config.Group, pending []config.CommandSpec, agg *result.RunResult, +) { + var todo []config.CommandSpec + for _, s := range pending { + if s.Always { + todo = append(todo, s) + } + } + if len(todo) == 0 { + return + } + grace, cancel := context.WithTimeout(context.WithoutCancel(ctx), teardownGrace) + defer cancel() + for _, s := range todo { + out, err := e.runner.Run(grace, e.commandGroup(group, s), s, e.cfg.Env) + aggregate(agg, &out) if err != nil { - // Keep singular-group error strings identical to the pre-multi - // behavior; only decorate when there is a sequence to point into. - if len(commands) > 1 { - err = fmt.Errorf("command %d of %d: %w", i+1, len(commands), err) - } - return agg, err + e.log.Warn("always command failed after cancellation", + "group", group.Name, "command", s.Command, "err", err.Error()) } } - return agg, nil +} + +// aggregate folds one command's streams and duration into the sequence total. +func aggregate(agg, out *result.RunResult) { + agg.Stdout += out.Stdout + agg.Stderr += out.Stderr + agg.Output += out.Output + agg.DurationMs += out.DurationMs +} + +// commandGroup returns the single-command view of a group that the runner +// receives. Argv-form entries clear Shell so they can never reach a shell. +func (e *Engine) commandGroup(group *config.Group, s config.CommandSpec) *config.Group { + sg := *group + sg.Command = s.Command + sg.Params = s.Params + sg.Commands = nil + if !s.IsShell { + sg.Shell = "" + } + return &sg } // cacheLookup returns the stored entry when caching is enabled for the group, diff --git a/internal/engine/failurepolicy_test.go b/internal/engine/failurepolicy_test.go new file mode 100644 index 0000000..688a08a --- /dev/null +++ b/internal/engine/failurepolicy_test.go @@ -0,0 +1,224 @@ +package engine + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/quike/keepup/internal/config" + "github.com/quike/keepup/internal/result" + "github.com/quike/keepup/internal/template" +) + +// runSeq drives runSequence directly with a scripted runner so each failure +// policy can be asserted on the commands that actually ran. +func runSeq(t *testing.T, commands []config.CommandSpec, errs map[string]error) (*specRunner, error) { + t.Helper() + r := &specRunner{errs: errs, outputs: map[string]string{}} + e := New(&config.Config{Version: 2}, WithRunner(r)) + g := &config.Group{Name: "g", Commands: commands} + _, err := e.runSequence(context.Background(), g, commands) + return r, err +} + +func TestRunSequence_StopsAtFirstFailureByDefault(t *testing.T) { + t.Parallel() + boom := errors.New("boom") + r, err := runSeq(t, []config.CommandSpec{ + {Command: "setup"}, {Command: "test"}, {Command: "deploy"}, + }, map[string]error{"test": boom}) + + require.Error(t, err) + assert.ErrorIs(t, err, boom) + assert.Equal(t, []string{"setup", "test"}, r.commandSeq()) +} + +func TestRunSequence_AlwaysRunsAfterFailure(t *testing.T) { + t.Parallel() + boom := errors.New("boom") + r, err := runSeq(t, []config.CommandSpec{ + {Command: "setup"}, + {Command: "test"}, + {Command: "skipped"}, + {Command: "teardown", Always: true}, + }, map[string]error{"test": boom}) + + require.Error(t, err, "always must not forgive the original failure") + assert.ErrorIs(t, err, boom) + assert.Equal(t, []string{"setup", "test", "teardown"}, r.commandSeq(), + "entries after a failure are skipped unless always") +} + +func TestRunSequence_ContinueOnErrorTolerates(t *testing.T) { + t.Parallel() + r, err := runSeq(t, []config.CommandSpec{ + {Command: "setup"}, + {Command: "lint", ContinueOnError: true}, + {Command: "deploy"}, + }, map[string]error{"lint": errors.New("lint failed")}) + + require.NoError(t, err, "a tolerated failure must not fail the group") + assert.Equal(t, []string{"setup", "lint", "deploy"}, r.commandSeq()) +} + +// Fully-tolerated semantics: the group reports success, so its result must not +// carry the soft failure's exit code. +func TestRunSequence_ContinueOnErrorLeavesResultClean(t *testing.T) { + t.Parallel() + r := &specRunner{errs: map[string]error{"lint": errors.New("nope")}, outputs: map[string]string{}} + e := New(&config.Config{Version: 2}, WithRunner(r)) + commands := []config.CommandSpec{{Command: "lint", ContinueOnError: true}} + + out, err := e.runSequence(context.Background(), &config.Group{Name: "g"}, commands) + require.NoError(t, err) + assert.Zero(t, out.ExitCode) + assert.Equal(t, "ok", out.Status) +} + +func TestRunSequence_AlwaysFailureKeepsFirstError(t *testing.T) { + t.Parallel() + first := errors.New("the real cause") + r, err := runSeq(t, []config.CommandSpec{ + {Command: "test"}, + {Command: "teardown", Always: true}, + }, map[string]error{"test": first, "teardown": errors.New("cleanup also broke")}) + + require.Error(t, err) + assert.ErrorIs(t, err, first, "the original failure must win over the teardown's") + assert.NotContains(t, err.Error(), "cleanup also broke") + assert.Equal(t, []string{"test", "teardown"}, r.commandSeq()) +} + +func TestRunSequence_AlwaysWithContinueOnErrorSucceeds(t *testing.T) { + t.Parallel() + r, err := runSeq(t, []config.CommandSpec{ + {Command: "test"}, + {Command: "cleanup", Always: true, ContinueOnError: true}, + }, map[string]error{"cleanup": errors.New("cleanup broke")}) + + require.NoError(t, err, "a tolerated cleanup failure must not fail the group") + assert.Equal(t, []string{"test", "cleanup"}, r.commandSeq()) +} + +// A tolerated failure must not suppress a later real one. +func TestRunSequence_FailureAfterToleratedOneStillFails(t *testing.T) { + t.Parallel() + hard := errors.New("hard failure") + r, err := runSeq(t, []config.CommandSpec{ + {Command: "lint", ContinueOnError: true}, + {Command: "deploy"}, + }, map[string]error{"lint": errors.New("tolerated"), "deploy": hard}) + + require.Error(t, err) + assert.ErrorIs(t, err, hard) + assert.Equal(t, []string{"lint", "deploy"}, r.commandSeq()) +} + +func TestRunSequence_AlwaysRunsAfterCancellation(t *testing.T) { + t.Parallel() + r := &specRunner{outputs: map[string]string{}} + e := New(&config.Config{Version: 2}, WithRunner(r)) + commands := []config.CommandSpec{ + {Command: "work"}, + {Command: "skipped"}, + {Command: "teardown", Always: true}, + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := e.runSequence(ctx, &config.Group{Name: "g"}, commands) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled, "cancellation is still reported") + assert.Equal(t, []string{"teardown"}, r.commandSeq(), + "only always entries run once the context is dead") +} + +func TestRunSequence_CancellationWithNoAlwaysEntriesRunsNothing(t *testing.T) { + t.Parallel() + r := &specRunner{outputs: map[string]string{}} + e := New(&config.Config{Version: 2}, WithRunner(r)) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := e.runSequence(ctx, &config.Group{Name: "g"}, + []config.CommandSpec{{Command: "work"}, {Command: "more"}}) + require.ErrorIs(t, err, context.Canceled) + assert.Empty(t, r.commandSeq()) +} + +// cancelOnFailRunner fails one command and cancels the run at the same moment, +// reproducing "the command failed, then the group timed out" deterministically. +type cancelOnFailRunner struct { + failOn string + err error + cancel context.CancelFunc + seen []string +} + +func (r *cancelOnFailRunner) Run( + _ context.Context, g *config.Group, _ config.CommandSpec, _ map[string]string, +) (result.RunResult, error) { + r.seen = append(r.seen, g.Command) + if g.Command == r.failOn { + r.cancel() + return result.RunResult{Status: result.StatusOK, ExitCode: 1}, r.err + } + return result.RunResult{Status: result.StatusOK}, nil +} + +// An earlier real failure is more actionable than the cancellation that +// followed it, so it stays the reported error. +func TestRunSequence_EarlierFailureWinsOverCancellation(t *testing.T) { + t.Parallel() + boom := errors.New("the real cause") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + r := &cancelOnFailRunner{failOn: "work", err: boom, cancel: cancel} + e := New(&config.Config{Version: 2}, WithRunner(r)) + + _, err := e.runSequence(ctx, &config.Group{Name: "g"}, []config.CommandSpec{ + {Command: "work"}, + {Command: "teardown", Always: true}, + }) + require.Error(t, err) + assert.ErrorIs(t, err, boom) + assert.Equal(t, []string{"work", "teardown"}, r.seen, + "teardown still runs even though the context died") +} + +// Regression guard, not a new behavior: expandCommands copies the spec, so +// non-templated fields must reach the runner untouched. +func TestExpandCommands_CarriesFailurePolicy(t *testing.T) { + t.Parallel() + e := New(&config.Config{Version: 2}) + g := &config.Group{Name: "g", Commands: []config.CommandSpec{ + {Command: "a", ContinueOnError: true}, + {Command: "b", Always: true}, + }} + + got, err := e.expandCommands(g, template.Data{}) + require.NoError(t, err) + assert.True(t, got[0].ContinueOnError) + assert.True(t, got[1].Always) +} + +func TestRunSequence_OutputOfAlwaysEntryIsAggregated(t *testing.T) { + t.Parallel() + boom := errors.New("boom") + r := &specRunner{ + errs: map[string]error{"test": boom}, + outputs: map[string]string{"test": "testing\n", "teardown": "cleaning\n"}, + } + e := New(&config.Config{Version: 2}, WithRunner(r)) + commands := []config.CommandSpec{{Command: "test"}, {Command: "teardown", Always: true}} + + out, err := e.runSequence(context.Background(), &config.Group{Name: "g"}, commands) + require.Error(t, err) + assert.Contains(t, out.Output, "testing") + assert.Contains(t, out.Output, "cleaning") +} diff --git a/internal/result/result.go b/internal/result/result.go index a6a4631..66ddd3f 100644 --- a/internal/result/result.go +++ b/internal/result/result.go @@ -20,9 +20,9 @@ type RunResult struct { // matching the historical (pre-structured-outputs) capture behavior. // The `output "x"` template function returns strings.TrimSpace(Output). Output string `json:"output,omitempty"` - // ExitCode is the process exit code. Always 0 in stored results today - // (non-zero aborts the flow before storage). Laid down for a future - // soft-fail / continue-on-error feature. + // ExitCode is the process exit code. Still always 0 in stored results: a + // hard failure aborts the flow before storage, and a continue-on-error + // entry's failure is tolerated rather than recorded. ExitCode int `json:"exitCode,omitempty"` // DurationMs is wall-clock milliseconds for the command run. 0 for // skipped and cache-hit groups.