diff --git a/CHANGELOG.md b/CHANGELOG.md index c0a33a7..ec6fc01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## v6.2.1 + +- `--expect-state RUNNING` (or `PREPARED`, `CREATED`) passes for a run that ended after it + started. The CLI checks a run every 5 seconds, and a short run can go from being + prepared to its end between two checks; it failed with "completed, but running was + expected". A run the platform refused, which never started, still fails. + ## v6.2.0 - `experiment run` can do what the `steadybit/run-experiment` GitHub Action does, so that diff --git a/e2e/platform.sh b/e2e/platform.sh index 192af54..19ab716 100755 --- a/e2e/platform.sh +++ b/e2e/platform.sh @@ -105,7 +105,8 @@ trap cleanup EXIT echo "steadybit $(steadybit --version) against ${STEADYBIT_URL:-https://platform.steadybit.com}, team $TEAM" -experiment experiments/a.yml a 5s +# a runs long enough for a poll to see it running, which one check expects. +experiment experiments/a.yml a 15s experiment experiments/b.yml b 8s experiment long.yml long 90s diff --git a/internal/experiment/experiment.go b/internal/experiment/experiment.go index 7948a70..8c4d6e0 100644 --- a/internal/experiment/experiment.go +++ b/internal/experiment/experiment.go @@ -693,6 +693,23 @@ func keyByExternalID(ctx context.Context, c *platform.Client, externalID string) var terminal = map[string]bool{"FAILED": true, "ERRORED": true, "CANCELED": true, "COMPLETED": true} +// passedThrough tells whether a run that ended went through the state expected on the +// way: between two polls a short run can go from PREPARED to COMPLETED. A run refused +// while another one ran has no start, and never reached PREPARED or RUNNING. A reason +// belongs to the state it was seen with, so an expected one needs the state itself. +func passedThrough(run *RunResult, o WaitOptions) bool { + if !terminal[run.State] || o.ExpectReason != "" { + return false + } + switch o.ExpectState { + case "CREATED": + return true + case "PREPARED", "RUNNING": + return !run.Started.IsZero() + } + return false +} + // WaitOptions shape what `run --wait` does besides waiting. type WaitOptions struct { // Timeout cancels the run once it has taken this long; zero waits indefinitely. @@ -706,7 +723,8 @@ type WaitOptions struct { // Prefix starts each line about the run, telling runs apart when several go at once. Prefix string // ExpectState passes the run once it reaches this state, which need not be an end - // such as RUNNING, and fails it when it ends in another. Empty expects COMPLETED. + // such as RUNNING, and fails it when it ends in another. A run that ended passes a + // state it went through without a poll seeing it. Empty expects COMPLETED. ExpectState string // ExpectReason also requires the run's reason to be exactly this. ExpectReason string @@ -829,7 +847,7 @@ func wait(ctx context.Context, c *platform.Client, location string, o WaitOption } } } - if o.ExpectState != "" && run.State == o.ExpectState { + if o.ExpectState != "" && (run.State == o.ExpectState || passedThrough(run, o)) { if o.ExpectReason != "" && run.Reason != o.ExpectReason { return run, unexpected(fmt.Sprintf("Experiment %s (#%d) %s with reason %q, but the reason %q was expected", run.Key, run.ID, strings.ToLower(run.State), run.Reason, o.ExpectReason)) } diff --git a/internal/experiment/experiment_test.go b/internal/experiment/experiment_test.go index d0a9e19..bc2b2ae 100644 --- a/internal/experiment/experiment_test.go +++ b/internal/experiment/experiment_test.go @@ -1055,8 +1055,12 @@ func TestAParallelRunThatCannotStartIsReported(t *testing.T) { } // run is one poll of a run, for the expectation tests. -func runState(state, reason string) platformtest.Reply { - return platformtest.Reply{JSON: map[string]any{"id": 1, "key": "TST-1", "state": state, "reason": reason}} +func runState(state, reason string, started bool) platformtest.Reply { + run := map[string]any{"id": 1, "key": "TST-1", "state": state, "reason": reason} + if started { + run["started"] = "2026-09-29T16:21:07.81269Z" + } + return platformtest.Reply{JSON: run} } func TestExpectedStatesAndReasons(t *testing.T) { @@ -1066,12 +1070,20 @@ func TestExpectedStatesAndReasons(t *testing.T) { expect experiment.WaitOptions err string lastPolled int + started bool }{ "a failure that was expected passes": {states: []string{"RUNNING", "FAILED"}, reason: "Check failure.", expect: experiment.WaitOptions{ExpectState: "FAILED"}}, "RUNNING passes before the run ends": {states: []string{"CREATED", "RUNNING", "COMPLETED"}, expect: experiment.WaitOptions{ExpectState: "RUNNING"}, lastPolled: 2}, "another end fails, naming both": {states: []string{"COMPLETED"}, expect: experiment.WaitOptions{ExpectState: "FAILED"}, err: "Experiment TST-1 (#1) completed, but failed was expected"}, "the reason has to match exactly": {states: []string{"FAILED"}, reason: "Check failure.", expect: experiment.WaitOptions{ExpectState: "FAILED", ExpectReason: "Timeout."}, err: `Experiment TST-1 (#1) failed with reason "Check failure.", but the reason "Timeout." was expected`}, "without an expectation, as it always was": {states: []string{"FAILED"}, reason: "Check failure.", err: "Experiment TST-1 (#1) failed, reason: Check failure."}, + // Polls a few seconds apart can miss a state a short run went through. + "RUNNING passes when the run ended after it started": {states: []string{"COMPLETED"}, started: true, expect: experiment.WaitOptions{ExpectState: "RUNNING"}}, + "RUNNING passes when a started run failed": {states: []string{"FAILED"}, started: true, expect: experiment.WaitOptions{ExpectState: "RUNNING"}}, + "PREPARED passes when a started run was canceled": {states: []string{"CANCELED"}, started: true, expect: experiment.WaitOptions{ExpectState: "PREPARED"}}, + "RUNNING fails for a run that never started": {states: []string{"CANCELED"}, reason: "Another experiment was running.", expect: experiment.WaitOptions{ExpectState: "RUNNING"}, err: "Experiment TST-1 (#1) canceled, reason: Another experiment was running., but running was expected"}, + "a reason expected with RUNNING needs RUNNING itself": {states: []string{"COMPLETED"}, started: true, expect: experiment.WaitOptions{ExpectState: "RUNNING", ExpectReason: "x"}, err: "Experiment TST-1 (#1) completed, but running was expected"}, + "an end expected is not passed through": {states: []string{"COMPLETED"}, started: true, expect: experiment.WaitOptions{ExpectState: "FAILED"}, err: "Experiment TST-1 (#1) completed, but failed was expected"}, } { t.Run(name, func(t *testing.T) { p := platformtest.New(t) @@ -1082,7 +1094,7 @@ func TestExpectedStatesAndReasons(t *testing.T) { if i >= len(tc.states) { i = len(tc.states) - 1 } - return runState(tc.states[i], tc.reason) + return runState(tc.states[i], tc.reason, tc.started) }) _, err := platformtest.Stdout(t, func() error { @@ -1122,7 +1134,7 @@ func TestBusyRetriesWaitInsteadOfRunningInParallel(t *testing.T) { } return started(p, "TST-1", 1) }) - p.Reply("GET /api/experiments/executions/1", runState("COMPLETED", "")) + p.Reply("GET /api/experiments/executions/1", runState("COMPLETED", "", false)) out, err := platformtest.Stdout(t, func() error { return experiment.Run(ctx, p.Client, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, BusyRetries: 3}) @@ -1146,9 +1158,9 @@ func TestBusyRetriesAlsoCoverARunCanceledForAnother(t *testing.T) { var polls atomic.Int32 p.Handle("GET /api/experiments/executions/1", func(platformtest.Request) platformtest.Reply { if polls.Add(1) == 1 { - return runState("CANCELED", "The run was started via CLI, but another experiment was running in parallel.") + return runState("CANCELED", "The run was started via CLI, but another experiment was running in parallel.", false) } - return runState("COMPLETED", "") + return runState("COMPLETED", "", false) }) _, err := platformtest.Stdout(t, func() error { @@ -1165,9 +1177,9 @@ func TestExpectationRetriesRunTheExperimentAgain(t *testing.T) { var polls atomic.Int32 p.Handle("GET /api/experiments/executions/1", func(platformtest.Request) platformtest.Reply { if polls.Add(1) <= 2 { - return runState("FAILED", "flaky") + return runState("FAILED", "flaky", false) } - return runState("COMPLETED", "") + return runState("COMPLETED", "", false) }) report := filepath.Join(t.TempDir(), "run.json") @@ -1200,7 +1212,7 @@ func TestRunByExternalID(t *testing.T) { return platformtest.Reply{JSON: map[string]any{"experiments": []any{}}} }) p.Reply("POST /api/experiments/TST-1/execute", started(p, "TST-1", 1)) - p.Reply("GET /api/experiments/executions/1", runState("COMPLETED", "")) + p.Reply("GET /api/experiments/executions/1", runState("COMPLETED", "", false)) run := func(o experiment.RunOptions) error { o.Yes, o.Wait = true, true _, err := platformtest.Stdout(t, func() error { return experiment.Run(ctx, p.Client, o) })