From df3d2c8ef70800c128a391da5b12b734274d6aea Mon Sep 17 00:00:00 2001 From: "antoine.choimet" <12182686+achoimet@users.noreply.github.com.> Date: Tue, 29 Sep 2026 17:16:47 +0200 Subject: [PATCH 1/3] feat: experiment run does what the run-experiment action does The steadybit/run-experiment GitHub Action has its own API client; to run it on the CLI instead, `experiment run` needs what it offers: - --expect-state passes once the run reaches a state, which need not be its end (RUNNING), and fails when it ends in another; --expect-reason also requires the reason. Without them a run has to complete, as before. - --expectation-retries runs the experiment again when a run did not end as expected, --expectation-retry-interval apart. - --busy-retries waits and tries again while another experiment runs, both when the platform refuses the run and when it cancels it right after accepting it, and never starts it in parallel instead, which --yes would do. The platform's rule spans teams, so this matters in a shared tenant. - --external-id without --template runs the experiment with that external id. - The JSON report gives each run's apiLocation, the action's executionUrl. - With --retries, the last attempt is kept on the platform, as the action does, so a run shows what was wrong. "Another experiment running" is now told apart before validation errors: the platform answers both with 422, so with --retries the former used to be retried as a validation error. Both run paths share one function that starts, waits and retries. The weekly platform test covers the new flags. --- CHANGELOG.md | 17 ++ README.md | 19 +- e2e/platform.sh | 17 ++ internal/cli/experiment.go | 9 + internal/experiment/experiment.go | 231 +++++++++++++++++++------ internal/experiment/experiment_test.go | 174 ++++++++++++++++++- internal/experiment/report.go | 29 ++-- 7 files changed, 428 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c89e7a..c0a33a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Changelog +## v6.2.0 + +- `experiment run` can do what the `steadybit/run-experiment` GitHub Action does, so that + the action can run on the CLI: + - `--expect-state` passes once the run reaches a state, which need not be its end, such + as `FAILED` for an experiment expected to find a weakness, or `RUNNING`, and fails when + it ends in another; `--expect-reason` also requires the run's reason. + - `--expectation-retries` and `--expectation-retry-interval` run the experiment again + when a run did not end as expected. + - `--busy-retries` and `--busy-retry-interval` wait and try again while another + experiment is running, whether the platform refuses the run or cancels it right after + accepting it, instead of asking, failing, or running in parallel as `--yes` would. + - `--external-id` without `--template` runs the experiment with that external id. + - The JSON report gives each run's `apiLocation`. +- With `--retries`, the last attempt at a run with validation errors is kept on the + platform, so the run shows what was wrong; the attempts before it are not. + ## v6.1.0 - `experiment run --parallel N` runs up to N of the experiments given with `-f` at once, diff --git a/README.md b/README.md index e852d6a..301f992 100644 --- a/README.md +++ b/README.md @@ -327,13 +327,18 @@ platform fills in with defaults are not reported as differences. still watches the run until it started, for up to 15 seconds, and fails when the platform canceled or errored it before it ran. A few options make it fit pipelines: -| Option | Does | -| ----------------------------- | ------------------------------------------------------------------------ | -| `--report steadybit.xml` | A JUnit report, one test case per step; `.json` for JSON | -| `--timeout 30m` | Cancels the run and fails when it has not ended in time | -| `--show-steps` | Prints each step's state as it changes | -| `--keep-running-on-interrupt` | Leaves the run going when the job is cancelled; by default it is stopped | -| `--parallel 3` | Runs up to 3 of the experiments at once; all are reported | +| Option | Does | +| ----------------------------- | ------------------------------------------------------------------------- | +| `--report steadybit.xml` | A JUnit report, one test case per step; `.json` for JSON | +| `--timeout 30m` | Cancels the run and fails when it has not ended in time | +| `--show-steps` | Prints each step's state as it changes | +| `--keep-running-on-interrupt` | Leaves the run going when the job is cancelled; by default it is stopped | +| `--parallel 3` | Runs up to 3 of the experiments at once; all are reported | +| `--expect-state FAILED` | Passes once the run reaches this state, and fails when it ends in another | +| `--expect-reason "…"` | Also requires the run's reason to be exactly this | +| `--expectation-retries 2` | Runs the experiment again when a run did not end as expected | +| `--busy-retries 3` | Waits and tries again while another experiment runs, instead of failing | +| `--external-id shop-latency` | Runs the experiment with this external id, instead of `-k` | In GitHub Actions a summary of every run is added to the job summary. diff --git a/e2e/platform.sh b/e2e/platform.sh index b04b06d..7a3ffdc 100755 --- a/e2e/platform.sh +++ b/e2e/platform.sh @@ -56,6 +56,7 @@ experiment() { # file name duration cat >"$1" </dev/null 2>&1 && + grep -Eq '\"state\": *\"RUNNING\"' expect.json && grep -q '\"apiLocation\"' expect.json +" +until_run_is "$A" COMPLETED CANCELED +check "a run that ends otherwise than expected fails" exits_with 1 steadybit experiment run -k "$A" --yes --allowParallel --expect-state FAILED +# The long run still goes, so both tries are refused: what is checked is that the CLI tries +# again instead of failing at once or, as --yes would otherwise do, running in parallel. +# Waiting for the platform to be free would depend on what other suites run at the time. +check "--busy-retries tries again while another experiment runs" sh -c " + steadybit experiment run -k $A --yes --busy-retries 1 --busy-retry-interval 5s >busy.log 2>&1 + status=\$? + grep -q 'trying again in 5s (1/1)' busy.log && [ \$status -eq 1 ] +" check "execution list prints the platform's runs as JSON" sh -c " [ \"\$(steadybit execution list --team $TEAM --limit 2 --jq length 2>/dev/null)\" -ge 1 ] " diff --git a/internal/cli/experiment.go b/internal/cli/experiment.go index bda68c8..7c32764 100644 --- a/internal/cli/experiment.go +++ b/internal/cli/experiment.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "strings" + "time" "github.com/spf13/cobra" "github.com/steadybit/cli/v6/internal/experiment" @@ -75,6 +76,7 @@ func newExperimentRun() *cobra.Command { "steadybit experiment run -f ./experiments -R --yes --timeout 30m --report steadybit.xml", "steadybit experiment run -f ./experiments -R --yes --parallel 3 --report steadybit.xml", "steadybit experiment run --template d7e65100-1d20-4980-be87-c351704910b8 --team ADM -p CLUSTER=prod", + "steadybit experiment run --external-id shop-latency --yes --expect-state FAILED --busy-retries 3", ), RunE: withClient(func(ctx context.Context, c *platform.Client, _ []string) error { o.Wait = !noWait @@ -90,6 +92,12 @@ func newExperimentRun() *cobra.Command { f.BoolVar(&o.AllowParallel, "allowParallel", false, "Skip the prompt warning about another experiment running and allow always parallel execution.") f.IntVar(&o.Retries, "retries", 0, "Number of retries when the experiment fails validation (e.g., missing targets). 0 means no retry.") f.IntVar(&o.RetryInterval, "retryInterval", 10, "Interval in seconds between retries.") + f.IntVar(&o.BusyRetries, "busy-retries", 0, "When another experiment is running and running in parallel is not allowed: try again this many times instead of asking, or failing in a pipeline.") + f.DurationVar(&o.BusyRetryInterval, "busy-retry-interval", 30*time.Second, "How long to wait before trying again while another experiment is running.") + f.StringVar(&o.ExpectState, "expect-state", "", "With waiting: pass once the run reaches this state, such as FAILED or RUNNING, and fail when it ends in another. (default: COMPLETED)") + f.StringVar(&o.ExpectReason, "expect-reason", "", "With waiting: also require the run's reason to be exactly this.") + f.IntVar(&o.ExpectationRetries, "expectation-retries", 0, "With waiting: run the experiment again this many times when a run does not end as expected.") + f.DurationVar(&o.ExpectationRetryInterval, "expectation-retry-interval", time.Minute, "How long to wait before running the experiment again after a run did not end as expected.") f.IntVar(&o.Parallel, "parallel", 1, "How many of the experiments given with -f to run at once. Each waits for its own run; the command fails if any fails.") f.DurationVar(&o.Timeout, "timeout", 0, `With waiting: cancel the run and fail when it has not ended after this long, e.g. "15m".`) f.BoolVar(&o.KeepRunningOnInterrupt, "keep-running-on-interrupt", false, "With waiting: leave the run going when the CLI is interrupted, instead of cancelling it.") @@ -97,6 +105,7 @@ func newExperimentRun() *cobra.Command { f.StringVar(&o.Report, "report", "", `With waiting: write a JUnit report of the runs to this file, or JSON if it ends in ".json".`) f.Var(executionVariables, "execution-variable", "With --template: a variable for this run only, overriding experiment and environment variables. Repeat for more.") addTemplateFlags(cmd, &o.TemplateOptions) + cmd.Flags().Lookup("external-id").Usage = "Without --template: run the experiment with this external id. With --template: an identifier of your own; using the same one again updates the experiment it created before." // --key with one --file updates that experiment from the file and runs it, as it did. variadic(cmd, "file") return cmd diff --git a/internal/experiment/experiment.go b/internal/experiment/experiment.go index 6496585..7948a70 100644 --- a/internal/experiment/experiment.go +++ b/internal/experiment/experiment.go @@ -22,6 +22,7 @@ import ( "os" "path" "path/filepath" + "slices" "strconv" "strings" "sync" @@ -273,6 +274,14 @@ type RunOptions struct { RetryInterval int // Parallel is how many runs go at once; 0 or 1 runs them one after another. Parallel int + // BusyRetries tries a run again, BusyRetryInterval apart, when another experiment is + // running and running in parallel is not allowed, instead of asking or failing. + BusyRetries int + BusyRetryInterval time.Duration + // ExpectationRetries runs an experiment again, ExpectationRetryInterval apart, when + // its run did not end as expected. + ExpectationRetries int + ExpectationRetryInterval time.Duration WaitOptions // Report is a file to write a JUnit (or, for .json, JSON) report of the runs to. Report string @@ -295,7 +304,6 @@ func Run(ctx context.Context, c *platform.Client, o RunOptions) error { } } - persist := o.Retries == 0 // Each run names its experiment in the question about running in parallel, as the // TypeScript CLI did. runs := []runner{} @@ -303,7 +311,7 @@ func Run(ctx context.Context, c *platform.Client, o RunOptions) error { case o.Template != "" && o.Key != "": return errors.New("--key cannot be combined with --template. Use `experiment apply --template -k` to update it.") case o.Template != "": - runs = append(runs, runner{"this one", func(parallel bool) (started, error) { return runTemplate(ctx, c, o, parallel, persist) }, false}) + runs = append(runs, runner{"this one", func(parallel, persist bool) (started, error) { return runTemplate(ctx, c, o, parallel, persist) }, false}) case len(o.Files) > 0: files, err := ResolveFiles(o.Files, o.Recursive) if err != nil { @@ -313,14 +321,41 @@ func Run(ctx context.Context, c *platform.Client, o RunOptions) error { return errors.New("If --key is specified, at most one --file can be specified.") } for _, file := range files { - runs = append(runs, runner{fileExperimentName(file), func(parallel bool) (started, error) { return runFile(ctx, c, o, file, parallel, persist) }, false}) + runs = append(runs, runner{fileExperimentName(file), func(parallel, persist bool) (started, error) { return runFile(ctx, c, o, file, parallel, persist) }, false}) } case o.Key != "": - runs = append(runs, runner{o.Key, func(parallel bool) (started, error) { return runKey(ctx, c, o.Key, parallel, persist) }, true}) + if o.ExternalID != "" { + return errors.New("--external-id finds the experiment to run; leave out --key.") + } + runs = append(runs, runner{o.Key, func(parallel, persist bool) (started, error) { return runKey(ctx, c, o.Key, parallel, persist) }, true}) + case o.ExternalID != "": + // Without --template, the external id names an experiment that exists, as the + // run-experiment action's externalId does. + key, err := keyByExternalID(ctx, c, o.ExternalID) + if err != nil { + return err + } + runs = append(runs, runner{key, func(parallel, persist bool) (started, error) { return runKey(ctx, c, key, parallel, persist) }, true}) default: return errors.New("Either --key, --file or --template must be specified.") } + if o.ExpectReason != "" && o.ExpectState == "" { + o.ExpectState = "COMPLETED" + } + if o.ExpectState != "" { + o.ExpectState = strings.ToUpper(o.ExpectState) + if !slices.Contains(runStates, o.ExpectState) { + return fmt.Errorf("--expect-state must be one of %s, not '%s'.", strings.Join(runStates, ", "), o.ExpectState) + } + } + if !o.Wait && (o.ExpectState != "" || o.ExpectationRetries > 0) { + return errors.New("--expect-state, --expect-reason and --expectation-retries need to wait for the run; remove --no-wait.") + } + if o.Retries < 0 || o.BusyRetries < 0 || o.ExpectationRetries < 0 { + return errors.New("--retries, --busy-retries and --expectation-retries cannot be negative.") + } + if o.Parallel < 0 { return errors.New("--parallel cannot be negative.") } @@ -352,31 +387,72 @@ func Run(ctx context.Context, c *platform.Client, o RunOptions) error { return runConcurrently(ctx, c, o, runs, report, &finished) } for _, r := range runs { - result, err := withRetries(o, r.what, "", r.run) + done, err := runUntilExpected(ctx, c, o, r, "", func(result started) { printStarted(result, r.keyLast) }) + if done != nil { + finished = append(finished, done) + } if err != nil { return errors.Join(err, report()) } - printStarted(result, r.keyLast) - if o.Wait && result.APILocation != "" { - done, err := wait(ctx, c, result.APILocation, o.WaitOptions) - if done != nil { - done.UILocation = result.UILocation - finished = append(finished, done) - } + } + return report() +} + +// canceledForAnother is a run the platform accepted and then canceled, because another +// experiment was running. +func canceledForAnother(run *RunResult) bool { + return run.State == "CANCELED" && strings.Contains(run.Reason, "another experiment was running") +} + +// runUntilExpected starts one run and waits for it. It runs the experiment again when +// --expectation-retries asks for it and the run did not end as expected, and, with +// --busy-retries, when the platform canceled it because another experiment was running. +// It returns the last run, for the report, or nil when nothing started or --no-wait +// left a run going. +func runUntilExpected(ctx context.Context, c *platform.Client, o RunOptions, r runner, prefix string, onStart func(started)) (*RunResult, error) { + busy := 0 + for attempt := 0; ; attempt++ { + result, err := withRetries(o, r.what, prefix, r.run) + if err != nil { + return nil, err + } + onStart(result) + if result.APILocation == "" { + return nil, nil + } + if !o.Wait { + // Only a run that failed the check is reported: one still running has no result yet. + run, err := checkStarted(ctx, c, result.APILocation) if err != nil { - return errors.Join(err, report()) - } - } else if result.APILocation != "" { - // Only a run that failed the check is reported: one still running has no - // result yet. - if run, err := checkStarted(ctx, c, result.APILocation); err != nil { - run.UILocation = result.UILocation - finished = append(finished, run) - return errors.Join(err, report()) + run.UILocation, run.APILocation = result.UILocation, result.APILocation + return run, err } + return nil, nil + } + waitOptions := o.WaitOptions + if prefix != "" { + waitOptions.Prefix = "[" + result.Key + "] " + } + done, err := wait(ctx, c, result.APILocation, waitOptions) + if done != nil { + done.UILocation, done.APILocation = result.UILocation, result.APILocation + } + if err == nil || done == nil || !errors.Is(err, ErrUnexpected) || interrupt.Interrupted() { + return done, err + } + if !o.AllowParallel && canceledForAnother(done) && busy < o.BusyRetries { + busy++ + fmt.Printf("%sAnother experiment is running, trying again in %s (%d/%d).\n", prefix, o.BusyRetryInterval, busy, o.BusyRetries) + time.Sleep(o.BusyRetryInterval) + attempt-- + continue + } + if attempt >= o.ExpectationRetries { + return done, err } + fmt.Printf("%sExperiment run %d did not end as expected (attempt %d/%d). Running it again in %s.\n", prefix, done.ID, attempt+1, o.ExpectationRetries+1, o.ExpectationRetryInterval) + time.Sleep(o.ExpectationRetryInterval) } - return report() } // A run by key alone printed its locations before the key, one from a file after. @@ -419,25 +495,17 @@ func runConcurrently(ctx context.Context, c *platform.Client, o RunOptions, runs unreported(i, "CANCELED", errors.New("not started, the command was interrupted")) return } - result, err := withRetries(o, r.what, "["+r.what+"] ", r.run) - if err != nil { - unreported(i, "ERRORED", err) - return - } - printing.Lock() - printStarted(result, r.keyLast) - printing.Unlock() - if result.APILocation == "" { - return - } - waitOptions := o.WaitOptions - waitOptions.Prefix = "[" + result.Key + "] " - done, err := wait(ctx, c, result.APILocation, waitOptions) + done, err := runUntilExpected(ctx, c, o, r, "["+r.what+"] ", func(result started) { + printing.Lock() + defer printing.Unlock() + printStarted(result, r.keyLast) + }) if done == nil { - unreported(i, "ERRORED", err) + if err != nil { + unreported(i, "ERRORED", err) + } return } - done.UILocation = result.UILocation results[i] = done errs[i] = err }() @@ -454,18 +522,23 @@ func runConcurrently(ctx context.Context, c *platform.Client, o RunOptions, runs // runner is one run to start: how the question about running in parallel names it, how // to start it, and where its key goes in what is printed. type runner struct { - what string - run func(parallel bool) (started, error) + what string + // run starts it; persist keeps a run the platform refused, so that its errors can be + // seen in the platform. + run func(parallel, persist bool) (started, error) keyLast bool } // withRetries retries validation errors, which clear up once targets appear, and offers -// a parallel run when another experiment is already running. +// a parallel run when another experiment is already running, or, with --busy-retries, +// waits for it instead. Only the last attempt at a run with validation errors is kept on +// the platform, so that it shows what was wrong without the attempts before it. // The prefix starts its messages, telling runs apart when several start at once. -func withRetries(o RunOptions, what, prefix string, run func(parallel bool) (started, error)) (started, error) { +func withRetries(o RunOptions, what, prefix string, run func(parallel, persist bool) (started, error)) (started, error) { parallel := o.AllowParallel + busy := 0 for attempt := 0; ; attempt++ { - result, err := run(parallel) + result, err := run(parallel, attempt >= o.Retries) if err == nil { return result, nil } @@ -473,12 +546,18 @@ func withRetries(o RunOptions, what, prefix string, run func(parallel bool) (sta if !errors.As(err, &apiErr) { return result, err } - if apiErr.Status == http.StatusUnprocessableEntity && attempt < o.Retries { - fmt.Printf("%sExperiment has validation errors (attempt %d/%d). Retrying in %ds...\n", prefix, attempt+1, o.Retries+1, o.RetryInterval) - time.Sleep(time.Duration(o.RetryInterval) * time.Second) - continue - } if !parallel && apiErr.ProblemType() == anotherExperimentRunning { + if busy < o.BusyRetries { + busy++ + fmt.Printf("%sAnother experiment is running, trying again in %s (%d/%d).\n", prefix, o.BusyRetryInterval, busy, o.BusyRetries) + time.Sleep(o.BusyRetryInterval) + attempt-- + continue + } + // Waiting was asked for; starting in parallel after all is what it rules out. + if o.BusyRetries > 0 { + return result, platform.Failed(err, "Failed to execute experiment, another one was still running after %d tries", o.BusyRetries) + } ok := o.Yes if !ok { // Its own error: the platform's is what a "no" reports. @@ -493,6 +572,11 @@ func withRetries(o RunOptions, what, prefix string, run func(parallel bool) (sta continue } } + if apiErr.Status == http.StatusUnprocessableEntity && attempt < o.Retries { + fmt.Printf("%sExperiment has validation errors (attempt %d/%d). Retrying in %ds...\n", prefix, attempt+1, o.Retries+1, o.RetryInterval) + time.Sleep(time.Duration(o.RetryInterval) * time.Second) + continue + } return result, platform.Failed(err, "Failed to execute experiment") } } @@ -582,6 +666,31 @@ func runTemplate(ctx context.Context, c *platform.Client, o RunOptions, parallel // PollInterval is how often --wait asks for the state of a run. Tests shorten it. var PollInterval = 5 * time.Second +// runStates are the states a run goes through, which --expect-state can name. +var runStates = []string{"CREATED", "PREPARED", "RUNNING", "FAILED", "CANCELED", "COMPLETED", "ERRORED"} + +// keyByExternalID finds the experiment with an external id; exactly one must have it. +func keyByExternalID(ctx context.Context, c *platform.Client, externalID string) (string, error) { + var list struct { + Experiments []struct { + Key string `json:"key"` + } `json:"experiments"` + } + ids := []string{externalID} + resp, err := c.GetExperiments(ctx, &api.GetExperimentsParams{ExternalId: &ids}) + if _, err := platform.Decode(resp, err, &list); err != nil { + return "", platform.Failed(err, "Failed to find the experiment with external id %s", externalID) + } + switch len(list.Experiments) { + case 0: + return "", fmt.Errorf("No experiment has the external id '%s'.", externalID) + case 1: + default: + return "", fmt.Errorf("%d experiments have the external id '%s'; run one of them with --key.", len(list.Experiments), externalID) + } + return list.Experiments[0].Key, nil +} + var terminal = map[string]bool{"FAILED": true, "ERRORED": true, "CANCELED": true, "COMPLETED": true} // WaitOptions shape what `run --wait` does besides waiting. @@ -596,6 +705,11 @@ type WaitOptions struct { ShowSteps bool // 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. + ExpectState string + // ExpectReason also requires the run's reason to be exactly this. + ExpectReason string // Steps asks the platform for the steps of the run, which reports need. Steps bool } @@ -715,6 +829,12 @@ func wait(ctx context.Context, c *platform.Client, location string, o WaitOption } } } + if o.ExpectState != "" && run.State == o.ExpectState { + 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)) + } + return run, nil + } if !terminal[run.State] { if !deadline.IsZero() && time.Now().After(deadline) { cancel(fmt.Sprintf("Experiment run %d did not end within %s", run.ID, o.Timeout)) @@ -728,13 +848,26 @@ func wait(ctx context.Context, c *platform.Client, location string, o WaitOption } continue } + if o.ExpectState != "" { + return run, unexpected(notCompleted(run).Error() + ", but " + strings.ToLower(o.ExpectState) + " was expected") + } if run.State != "COMPLETED" { - return run, notCompleted(run) + return run, unexpected(notCompleted(run).Error()) } return run, nil } } +// ErrUnexpected marks a run that did not end as expected, by --expect-state and +// --expect-reason or by completing, which --expectation-retries runs again. +var ErrUnexpected = errors.New("the run did not end as expected") + +// unexpected is such a run's error: its message says how it ended, and it is ErrUnexpected. +type unexpected string + +func (e unexpected) Error() string { return string(e) } +func (unexpected) Is(target error) bool { return target == ErrUnexpected } + func notCompleted(run *RunResult) error { reason := "" if run.Reason != "" { diff --git a/internal/experiment/experiment_test.go b/internal/experiment/experiment_test.go index 2771b56..d0a9e19 100644 --- a/internal/experiment/experiment_test.go +++ b/internal/experiment/experiment_test.go @@ -191,7 +191,9 @@ func TestAFailedRunFailsTheCommand(t *testing.T) { assert.EqualError(t, err, "Experiment TST-1 (#1) failed, reason: hypothesis violated") } -func TestRunRetriesValidationErrorsWithoutPersistingThem(t *testing.T) { +// Only the last attempt is kept on the platform, so that it shows what was wrong without +// the attempts before it, as the run-experiment action does. +func TestRunRetriesValidationErrorsKeepingOnlyTheLast(t *testing.T) { p := platformtest.New(t) stillRunning(p) var calls atomic.Int32 @@ -208,9 +210,11 @@ func TestRunRetriesValidationErrorsWithoutPersistingThem(t *testing.T) { require.NoError(t, err) assert.Contains(t, out, "Experiment has validation errors (attempt 1/3). Retrying in 0s...") + var persisted []string for _, r := range p.Requests("POST /api/experiments/TST-1/execute") { - assert.Equal(t, []string{"false"}, r.Query["forcePersist"]) + persisted = append(persisted, r.Query["forcePersist"][0]) } + assert.Equal(t, []string{"false", "false", "true"}, persisted) } func TestRunGivesUpAfterTheLastRetry(t *testing.T) { @@ -1049,3 +1053,169 @@ func TestAParallelRunThatCannotStartIsReported(t *testing.T) { assert.Equal(t, "TST-2", runs[1]["key"]) assert.Equal(t, "COMPLETED", runs[1]["state"]) } + +// 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 TestExpectedStatesAndReasons(t *testing.T) { + for name, tc := range map[string]struct { + states []string + reason string + expect experiment.WaitOptions + err string + lastPolled int + }{ + "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."}, + } { + t.Run(name, func(t *testing.T) { + p := platformtest.New(t) + p.Reply("POST /api/experiments/TST-1/execute", started(p, "TST-1", 1)) + var polls atomic.Int32 + p.Handle("GET /api/experiments/executions/1", func(platformtest.Request) platformtest.Reply { + i := int(polls.Add(1)) - 1 + if i >= len(tc.states) { + i = len(tc.states) - 1 + } + return runState(tc.states[i], tc.reason) + }) + + _, err := platformtest.Stdout(t, func() error { + return experiment.Run(ctx, p.Client, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, WaitOptions: tc.expect}) + }) + + if tc.err == "" { + require.NoError(t, err) + } else { + assert.EqualError(t, err, tc.err) + } + if tc.lastPolled > 0 { + assert.EqualValues(t, tc.lastPolled, polls.Load(), "stops polling once the state is reached") + } + }) + } +} + +// The platform refuses a run while another one goes; --busy-retries waits instead of +// starting in parallel, which --yes would otherwise do. +func TestBusyRetriesWaitInsteadOfRunningInParallel(t *testing.T) { + busy := platformtest.Reply{Status: http.StatusUnprocessableEntity, Body: `{"type":"https://steadybit.com/problems/another-experiment-running-exception","title":"Another experiment is running"}`} + for name, tc := range map[string]struct { + refusals int + err string + }{ + "it starts once the other has ended": {refusals: 2}, + "it gives up after its tries": {refusals: 5, err: "Failed to execute experiment, another one was still running after 3 tries"}, + } { + t.Run(name, func(t *testing.T) { + p := platformtest.New(t) + var calls atomic.Int32 + p.Handle("POST /api/experiments/TST-1/execute", func(r platformtest.Request) platformtest.Reply { + assert.Equal(t, []string{"false"}, r.Query["allowParallel"]) + if int(calls.Add(1)) <= tc.refusals { + return busy + } + return started(p, "TST-1", 1) + }) + p.Reply("GET /api/experiments/executions/1", runState("COMPLETED", "")) + + out, err := platformtest.Stdout(t, func() error { + return experiment.Run(ctx, p.Client, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, BusyRetries: 3}) + }) + + if tc.err == "" { + require.NoError(t, err) + assert.Contains(t, out, "Another experiment is running, trying again in 0s (2/3).") + } else { + assert.ErrorContains(t, err, tc.err) + assert.EqualValues(t, 4, calls.Load(), "the first try and three more") + } + }) + } +} + +// The platform may also accept a run and cancel it right away for the same reason. +func TestBusyRetriesAlsoCoverARunCanceledForAnother(t *testing.T) { + p := platformtest.New(t) + p.Reply("POST /api/experiments/TST-1/execute", started(p, "TST-1", 1)) + 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("COMPLETED", "") + }) + + _, err := platformtest.Stdout(t, func() error { + return experiment.Run(ctx, p.Client, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, BusyRetries: 1}) + }) + + require.NoError(t, err) + assert.Len(t, p.Requests("POST /api/experiments/TST-1/execute"), 2) +} + +func TestExpectationRetriesRunTheExperimentAgain(t *testing.T) { + p := platformtest.New(t) + p.Reply("POST /api/experiments/TST-1/execute", started(p, "TST-1", 1)) + 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("COMPLETED", "") + }) + report := filepath.Join(t.TempDir(), "run.json") + + out, err := platformtest.Stdout(t, func() error { + return experiment.Run(ctx, p.Client, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, ExpectationRetries: 2, Report: report}) + }) + + require.NoError(t, err) + assert.Len(t, p.Requests("POST /api/experiments/TST-1/execute"), 3) + assert.Contains(t, out, "Experiment run 1 did not end as expected (attempt 1/3). Running it again in 0s.") + // The report holds the last run, with the API location run-experiment's output gives. + content, err := os.ReadFile(report) + require.NoError(t, err) + var runs []map[string]any + require.NoError(t, json.Unmarshal(content, &runs)) + require.Len(t, runs, 1) + assert.Equal(t, "COMPLETED", runs[0]["state"]) + assert.Equal(t, "https://elsewhere.example.com/api/experiments/executions/1", runs[0]["apiLocation"]) +} + +func TestRunByExternalID(t *testing.T) { + p := platformtest.New(t) + p.Handle("GET /api/experiments", func(r platformtest.Request) platformtest.Reply { + switch r.Query["externalId"][0] { + case "shop": + return platformtest.Reply{JSON: map[string]any{"experiments": []any{map[string]any{"key": "TST-1"}}}} + case "twice": + return platformtest.Reply{JSON: map[string]any{"experiments": []any{map[string]any{"key": "TST-1"}, map[string]any{"key": "TST-2"}}}} + } + 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", "")) + 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) }) + return err + } + + require.NoError(t, run(experiment.RunOptions{TemplateOptions: experiment.TemplateOptions{ExternalID: "shop"}})) + assert.EqualError(t, run(experiment.RunOptions{TemplateOptions: experiment.TemplateOptions{ExternalID: "gone"}}), "No experiment has the external id 'gone'.") + assert.EqualError(t, run(experiment.RunOptions{TemplateOptions: experiment.TemplateOptions{ExternalID: "twice"}}), "2 experiments have the external id 'twice'; run one of them with --key.") + assert.EqualError(t, run(experiment.RunOptions{Key: "TST-1", TemplateOptions: experiment.TemplateOptions{ExternalID: "shop"}}), "--external-id finds the experiment to run; leave out --key.") +} + +func TestExpectationsNeedWaitingAndARealState(t *testing.T) { + err := experiment.Run(ctx, nil, experiment.RunOptions{Key: "TST-1", Yes: true, Wait: true, WaitOptions: experiment.WaitOptions{ExpectState: "done"}}) + assert.EqualError(t, err, "--expect-state must be one of CREATED, PREPARED, RUNNING, FAILED, CANCELED, COMPLETED, ERRORED, not 'DONE'.") + err = experiment.Run(ctx, nil, experiment.RunOptions{Key: "TST-1", Yes: true, WaitOptions: experiment.WaitOptions{ExpectState: "FAILED"}}) + assert.EqualError(t, err, "--expect-state, --expect-reason and --expectation-retries need to wait for the run; remove --no-wait.") +} diff --git a/internal/experiment/report.go b/internal/experiment/report.go index 9d46d13..e3c41b6 100644 --- a/internal/experiment/report.go +++ b/internal/experiment/report.go @@ -22,7 +22,10 @@ type RunResult struct { Started time.Time `json:"started"` Ended time.Time `json:"ended"` UILocation string `json:"uiLocation,omitempty"` - Steps []Step `json:"steps"` + // APILocation is where the platform's API serves the run, as run-experiment's + // executionUrl output names it. + APILocation string `json:"apiLocation,omitempty"` + Steps []Step `json:"steps"` } type Step struct { @@ -45,14 +48,16 @@ func duration(from, to time.Time) time.Duration { func parseRun(body []byte) (*RunResult, error) { var raw struct { - ID int64 `json:"id"` - Key string `json:"key"` - Name string `json:"name"` - State string `json:"state"` - Reason string `json:"reason"` - Started time.Time `json:"started"` - Ended time.Time `json:"ended"` - Steps []struct { + ID int64 `json:"id"` + Key string `json:"key"` + Name string `json:"name"` + State string `json:"state"` + Reason string `json:"reason"` + // Older platforms named it so; run-experiment read it first. + FailureReason string `json:"failureReason"` + Started time.Time `json:"started"` + Ended time.Time `json:"ended"` + Steps []struct { StepType string `json:"stepType"` ActionID string `json:"actionId"` CustomLabel string `json:"customLabel"` @@ -66,7 +71,11 @@ func parseRun(body []byte) (*RunResult, error) { if err := json.Unmarshal(body, &raw); err != nil { return nil, err } - run := &RunResult{ID: raw.ID, Key: raw.Key, Name: raw.Name, State: raw.State, Reason: raw.Reason, Started: raw.Started, Ended: raw.Ended} + reason := raw.FailureReason + if reason == "" { + reason = raw.Reason + } + run := &RunResult{ID: raw.ID, Key: raw.Key, Name: raw.Name, State: raw.State, Reason: reason, Started: raw.Started, Ended: raw.Ended} for _, s := range raw.Steps { name := s.CustomLabel switch { From 736184d6de0bbe02ae97c3472e034c82fee633b6 Mon Sep 17 00:00:00 2001 From: "antoine.choimet" <12182686+achoimet@users.noreply.github.com.> Date: Tue, 29 Sep 2026 17:17:28 +0200 Subject: [PATCH 2/3] ci: a pull request's platform test runs the code it changes A change to the platform test can use flags the latest release does not have yet, so on a pull request the CLI is built from the branch. --- .github/workflows/platform-e2e.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/platform-e2e.yml b/.github/workflows/platform-e2e.yml index 915eb44..acdd4e0 100644 --- a/.github/workflows/platform-e2e.yml +++ b/.github/workflows/platform-e2e.yml @@ -38,14 +38,15 @@ jobs: STEADYBIT_E2E_ENVIRONMENT: Global steps: - uses: actions/checkout@v7 - # The latest release through the action, as a pipeline installs it. - - if: ${{ !inputs.from-source }} + # The latest release through the action, as a pipeline installs it; a pull request + # tests its own code, which a change to the test may depend on. + - if: ${{ !inputs.from-source && github.event_name != 'pull_request' }} uses: ./ - - if: ${{ inputs.from-source }} + - if: ${{ inputs.from-source || github.event_name == 'pull_request' }} uses: actions/setup-go@v7 with: go-version-file: go.mod - - if: ${{ inputs.from-source }} + - if: ${{ inputs.from-source || github.event_name == 'pull_request' }} run: | go build -o "$RUNNER_TEMP/bin/steadybit" ./cmd/steadybit echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" From 7b24acc46dcae68fc1935884192ff84456b2d31a Mon Sep 17 00:00:00 2001 From: "antoine.choimet" <12182686+achoimet@users.noreply.github.com.> Date: Tue, 29 Sep 2026 17:20:06 +0200 Subject: [PATCH 3/3] test: platform checks show what the CLI printed when they fail The expected-state check threw the CLI's output away, so a failure in CI said nothing about why. It now runs through exits_with, which prints it, and the check on a run ending otherwise asserts the message, not only the exit code. --- e2e/platform.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/e2e/platform.sh b/e2e/platform.sh index 7a3ffdc..192af54 100755 --- a/e2e/platform.sh +++ b/e2e/platform.sh @@ -151,12 +151,17 @@ check "execution list --fail-on-match gates on that canceled run" exits_with 1 \ steadybit execution list --key "$A" --state CANCELED --ended-from "$(date -u +%F)" --limit 1 --fail-on-match # What the run-experiment action relies on: the experiment found by its external id, an # expected state reached before the end, and waiting while another experiment runs. -check "an expected state passes as soon as the run reaches it" sh -c " - steadybit experiment run --external-id $MARK-$RUN-a --yes --allowParallel --expect-state RUNNING --report expect.json >/dev/null 2>&1 && - grep -Eq '\"state\": *\"RUNNING\"' expect.json && grep -q '\"apiLocation\"' expect.json -" +check "the experiment is found by its external id and passes at the expected state" exits_with 0 \ + steadybit experiment run --external-id "$MARK-$RUN-a" --yes --allowParallel --expect-state RUNNING --report expect.json +check "the report has the state reached and the run's API location" sh -c ' + grep -Eq "\"state\": *\"RUNNING\"" expect.json && grep -q "\"apiLocation\"" expect.json || { cat expect.json; exit 1; } +' until_run_is "$A" COMPLETED CANCELED -check "a run that ends otherwise than expected fails" exits_with 1 steadybit experiment run -k "$A" --yes --allowParallel --expect-state FAILED +check "a run that ends otherwise than expected fails" sh -c " + steadybit experiment run -k $A --yes --allowParallel --expect-state FAILED >otherwise.log 2>&1 + status=\$? + grep -q 'but failed was expected' otherwise.log && [ \$status -eq 1 ] || { tail -n 5 otherwise.log; exit 1; } +" # The long run still goes, so both tries are refused: what is checked is that the CLI tries # again instead of failing at once or, as --yes would otherwise do, running in parallel. # Waiting for the platform to be free would depend on what other suites run at the time.