diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fff013..062316c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Top of tree +* `autoland` plans have a new step, `x `, which runs a command once + the PRs before it have landed and stops the land unless it exits 0 — for + automating what would otherwise be a manual `c` step, such as provisioning. + The command runs without a shell (use `x bash -c '...'` for pipes), from + the top of the checkout, with the landed SHA, the landed PRs, the branch, + the remote, and the target in `AUTOLAND_*` environment variables. + `--resume` runs a failed or interrupted command again, so commands should + be idempotent. The VS Code extension (0.3.0) highlights and formats `x` + steps. * `autoland` now merges each run of consecutive `l` steps (with no `w` or `c` step between them) as a GitHub stack, using GitHub's native [stacked PRs](https://docs.github.com/en/pull-requests/get-started/about-stacked-prs): diff --git a/README.md b/README.md index eb8a911..8e61f89 100644 --- a/README.md +++ b/README.md @@ -575,10 +575,12 @@ Options: autoland reaches that step (`Confirm "QA sign-off complete" is complete — ready to proceed?`). A bare `c` just prompts `Ready to proceed?`. When `autoland.default_workflow` is configured, the pre-filled plan already ends - with a `w ` step, which you can edit or delete. + with a `w ` step, which you can edit or delete. Command + steps (`x ` — run a command, and stop unless it succeeds) can go + there too; see [Running commands](#running-commands). - `--plan-file PATH`: Load the landing plan from a file instead of editing it interactively. The file uses the exact same format as the `-i` editor (`l` / - `w ` / `c [condition]` lines, with `#` comments and blank lines + `w ` / `c [condition]` / `x ` lines, with `#` comments and blank lines ignored — see [Example plan](#example-plan) below), so you can save a plan from `-i`, or write one by hand, and reuse it for repeatable/scripted runs. Plans conventionally use the `.autoland-plan` suffix. @@ -605,10 +607,10 @@ Options: `state_file`, `state_file_exists`, `lock_file`, `pid`, `last_saved` (ISO 8601), `abort_reason`, `resume_command`, `plan` (`total` / `done` / `remaining` counts and a `steps` list), and `other_runs`. Each step has - `number`, `type` (`land` / `workflow` / `confirm`), `outcome` (`done` / - `active` / `pending` / `failed`), `status`, `is_next` and `detail`, plus - `pr_number` / `pr_url` / `title`, `workflow`, or `condition` depending on - its type. If the state file can't be read, stdout stays empty and the error + `number`, `type` (`land` / `workflow` / `confirm` / `exec`), `outcome` + (`done` / `active` / `pending` / `failed`), `status`, `is_next` and + `detail`, plus `pr_number` / `pr_url` / `title`, `workflow`, `condition`, or + `command` depending on its type. If the state file can't be read, stdout stays empty and the error goes to stderr with a nonzero exit. - `--state-file PATH`: Override the checkpoint path (default: `~/.stack-pr/autoland/.json`). @@ -655,7 +657,7 @@ default_workflow = deploy.yaml ##### Merging as a stack By default, `autoland` lands each run of two or more consecutive `l` steps (no -`w` or `c` step between them) with GitHub's native [stacked +`w`, `c`, or `x` step between them) with GitHub's native [stacked PRs](https://docs.github.com/en/pull-requests/get-started/about-stacked-prs) instead of one PR at a time: @@ -673,8 +675,8 @@ instead of one PR at a time: every PR. So a plan of four bare `l` steps waits on the merge queue once instead of four -times. A `w` or `c` step ends a run, since it has to see the PRs below it land -first; `--dry-run` lists the runs a plan would merge as stacks. +times. A `w`, `c`, or `x` step ends a run, since it has to see the PRs below it +land first; `--dry-run` lists the runs a plan would merge as stacks. To keep PRs from merging together, put a `c` step between them in the plan, which splits the run there (`autoland` asks you to confirm before continuing). @@ -707,8 +709,9 @@ installed (`pipx install 'stack-pr[rich]'` or add the `rich` extra); otherwise With `-i`, `autoland` opens a plan in `$EDITOR`. Each non-comment line is one step, run top to bottom: `l ` lands that PR, `w ` waits for a -named GitHub Actions workflow to finish with the landed code, and -`c [condition]` pauses for manual confirmation (the condition is optional). For +named GitHub Actions workflow to finish with the landed code, +`c [condition]` pauses for manual confirmation (the condition is optional), and +`x ` runs a command (see [Running commands](#running-commands)). For a three-PR stack, a plan that lands the bottom PR, waits for a deploy, gets manual sign-off, then lands the rest looks like: @@ -720,6 +723,8 @@ manual sign-off, then lands the rest looks like: # c [condition] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. @@ -750,12 +755,53 @@ repeatable or scripted landings. Any path works, but `.autoland-plan` is the conventional suffix: it is what `-i` writes, and what editors key off to highlight the plan syntax (see [Editor support](#editor-support)). +##### Running commands + +An `x ` step runs a command once the PRs before it have landed, and +stops the land unless it exits 0 — so a manual step that would otherwise be a +`c` ("provision the new tables, then confirm") can be automated: + +``` +l 101 # Add the widgets table +x ./scripts/provision.sh --env staging +l 102 # Start writing widgets +x bash -c 'curl -fsS "$HEALTH_URL" | grep -q ok' +l 103 +``` + +- The command is split into words the way a POSIX shell would (quotes and + backslashes work), but it runs **without a shell**: no pipes, redirection, + globs, or `$VARIABLE` expansion. For those, run a shell yourself, as in + `x bash -c '...'`. +- As with every step, ` #` (a space, then `#`) starts a comment, even inside + quotes, so a command can't contain one. +- It runs from the top of the checkout being landed (the temporary worktree, + with `--branch`), with its output going straight to the terminal, and with + `autoland`'s own environment plus: + - `AUTOLAND_LANDED_SHA`: the commit of the code landed so far — the merge + commit of the last PR this run landed, or else the head of the target + branch (empty if it can't be found); + - `AUTOLAND_LANDED_PRS`: the PRs the plan has landed before this step, + space-separated in plan order, including any that landed in an earlier + run; + - `AUTOLAND_BRANCH`, `AUTOLAND_REMOTE`, `AUTOLAND_TARGET`: the branch whose + stack is landing, and the remote and branch it lands into. +- A nonzero exit status, a command killed by a signal, or a command that can't + be started (no such program, not executable) stops the land; `--status` + shows why. There is no timeout. +- `--resume` skips a command that succeeded and **runs a failed or interrupted + one again**, so commands should be safe to re-run (idempotent). Ctrl+C + while one runs stops it along with `autoland`. As with `w` and `c` steps, an + `x` step before an already-landed PR is assumed done, and `--replan` keeps a + finished one when the new plan has the same command after the same landed + PRs. + ##### Re-using a plan across a partially-landed stack Because each `l` step names the PR it lands, one plan file stays valid as the stack lands piece by piece. A PR that has already merged is no longer part of -the stack, so `autoland` skips its `l` step — along with any `w` and `c` steps -that come before it, since those must have run before that PR could merge: +the stack, so `autoland` skips its `l` step — along with any `w`, `c`, and `x` +steps that come before it, since those must have run before that PR could merge: ``` l 1 # PR #1, already landed upstream @@ -796,9 +842,9 @@ need when only the code changed. Either way it: the run continues in your terminal. 2. Rediscovers the stack, since the code may have changed, and parses the new plan against it. Landed PRs are skipped as usual. -3. Carries over each `w` and `c` step the previous run completed, provided the - new plan has the same step (same workflow, or same condition text) after the - same set of landed PRs. A step means "once these PRs have landed, this +3. Carries over each `w`, `c`, and `x` step the previous run completed, + provided the new plan has the same step (same workflow, condition text, or + command) after the same set of landed PRs. A step means "once these PRs have landed, this holds", so its result still stands. Adding, removing, or reordering steps keeps that credit; editing a step's text, or landing a different set of PRs before it, makes it run again. @@ -812,8 +858,8 @@ autoland, so you can check what a replan would do before committing to it. ##### Editor support [`editors/vscode`](editors/vscode) is a VS Code extension that highlights plan -files: the `l` / `w` / `c` keywords, PR numbers and URLs, workflow names, -confirmation conditions, and `#` comments — plus an error scope for steps +files: the `l` / `w` / `c` / `x` keywords, PR numbers and URLs, workflow +names, confirmation conditions, commands, and `#` comments — plus an error scope for steps `autoland` would reject, so a typo shows up before you run the plan. It also formats them: **Format Document** aligns trailing comments at column 30 (or past the longest `l`/`w` step in the block, when one runs that far) and tidies diff --git a/editors/vscode/CHANGELOG.md b/editors/vscode/CHANGELOG.md index 2e55720..d515456 100644 --- a/editors/vscode/CHANGELOG.md +++ b/editors/vscode/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 0.3.0 + +* Highlights `x ` steps, which run a command and stop the land + unless it exits 0: the `x` keyword, and the command as a string. A bare `x` + highlights as an error, since the parser rejects it. The formatter + collapses the gap after `x` like any other step, and, as for `c`, an `x` + step never sets the comment column. + ## 0.2.0 * The extension is now a formatter for the language: **Format Document** diff --git a/editors/vscode/README.md b/editors/vscode/README.md index a1a6d00..2fb8ffc 100644 --- a/editors/vscode/README.md +++ b/editors/vscode/README.md @@ -1,7 +1,7 @@ # stack-pr Autoland Plan — VS Code support Syntax highlighting and formatting for `stack-pr autoland` landing plans: the -`l` / `w` / `c` step files you edit with `autoland -i` or pass to +`l` / `w` / `c` / `x` step files you edit with `autoland -i` or pass to `autoland --plan-file`. See [`examples/example.autoland-plan`](examples/example.autoland-plan) for a @@ -11,25 +11,27 @@ plan exercising every step type. | Plan syntax | Scope | | --- | --- | -| `l`, `w`, `c` step keywords | `keyword.control..autoland-plan` | +| `l`, `w`, `c`, `x` step keywords | `keyword.control..autoland-plan` | | `l 123` — the PR number | `constant.numeric.pr-number.autoland-plan` | | `l https://…/pull/123` — the PR URL | `markup.underline.link.autoland-plan` | | `w deploy.yaml` — the workflow name | `entity.name.function.workflow.autoland-plan` | | `c QA sign-off complete` — the condition | `string.unquoted.condition.autoland-plan` | +| `x ./provision.sh staging` — the command | `string.unquoted.command.autoland-plan` | | `#` comments, whole-line and trailing | `comment.line.number-sign.autoland-plan` | | a step the parser would reject | `invalid.illegal.*.autoland-plan` | The `invalid` scopes mirror `parse_plan` in `stack_pr/autoland/plan.py`, so a plan that highlights as an error is a plan `autoland` will refuse to run: an `l` with something that is neither a PR number nor a PR URL, a `w` with no -workflow, or a line that is not a step at all. +workflow, an `x` with no command, or a line that is not a step at all. Two details worth knowing, both inherited from the parser: - A `#` only starts a comment when whitespace precedes it. `l #123` is a bare `l` followed by a comment (which is why `l #123` does not pin PR 123), while `c ship it#now` keeps the `#` inside the condition. -- A workflow name runs to the end of the step, spaces included. +- A workflow name runs to the end of the step, spaces included, and so does + an `x` command — so a ` #` ends the command even inside quotes. ## Formatting @@ -45,14 +47,14 @@ does — the formatted text parses into exactly the same steps: - **A long `l` or `w` step widens its block**: if one reaches into column 30, that block's comments move to four spaces past the longest such step, so the block still lines up internally without affecting the rest of the file. -- **`c` steps never set the column.** A confirm condition is free text that - routinely runs long, and letting it decide would drag every comment in the - block off to the right. Their comments are still aligned with everything - else; a condition already past the column keeps a single space before its +- **`c` and `x` steps never set the column.** A confirm condition is free + text and a command line can be anything, and both routinely run long; + letting them decide would drag every comment in the block off to the right. + Their comments are still aligned with everything else; a condition already past the column keeps a single space before its `#`, since losing that space would fold the comment into the condition. - **The gap after a step keyword is collapsed** to a single space, and steps are - unindented. The argument itself is left alone: a condition is free text, and a - workflow name may contain spaces. + unindented. The argument itself is left alone: a condition is free text, a + workflow name may contain spaces, and a command's spacing may be quoted. - **Whole-line comments keep their own indentation**, since the generated plan header is hand-aligned. They do not break up a block. - **Trailing whitespace goes**, and the file ends with exactly one newline. @@ -88,7 +90,7 @@ From this directory: ```bash # Package and install into VS Code. npx --yes @vscode/vsce package -code --install-extension autoland-plan-0.2.0.vsix +code --install-extension autoland-plan-0.3.0.vsix ``` Or, for development, symlink it into your extensions directory and reload: diff --git a/editors/vscode/examples/example.autoland-plan b/editors/vscode/examples/example.autoland-plan index 771858a..7e51a98 100644 --- a/editors/vscode/examples/example.autoland-plan +++ b/editors/vscode/examples/example.autoland-plan @@ -5,6 +5,8 @@ # c [condition] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. @@ -12,6 +14,7 @@ l 101 # Add /widgets API endpoint w deploy.yaml # wait for the deploy of PR #101 to finish c QA sign-off complete # pause until QA has signed off +x ./scripts/provision.sh staging # stop unless it succeeds l 102 # Wire up the widgets UI # A bare 'l' lands whatever PR is next in the stack, and a PR can also be diff --git a/editors/vscode/examples/generated.autoland-plan b/editors/vscode/examples/generated.autoland-plan index 044cd56..b1c39e5 100644 --- a/editors/vscode/examples/generated.autoland-plan +++ b/editors/vscode/examples/generated.autoland-plan @@ -5,6 +5,8 @@ # c [] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. @@ -12,5 +14,6 @@ l 101 # Add /widgets API endpoint w deploy.yaml c QA sign-off complete +x ./scripts/provision.sh --env staging l 1024 # Wire up the widgets UI l 103 diff --git a/editors/vscode/package.json b/editors/vscode/package.json index 27b6c41..f0f419a 100644 --- a/editors/vscode/package.json +++ b/editors/vscode/package.json @@ -1,8 +1,8 @@ { "name": "autoland-plan", "displayName": "stack-pr Autoland Plan", - "description": "Syntax highlighting and formatting for stack-pr autoland plan files (l / w / c steps).", - "version": "0.2.0", + "description": "Syntax highlighting and formatting for stack-pr autoland plan files (l / w / c / x steps).", + "version": "0.3.0", "publisher": "micahjsmith", "license": "MIT", "repository": { diff --git a/editors/vscode/src/format.js b/editors/vscode/src/format.js index 2bdb435..66dfc76 100644 --- a/editors/vscode/src/format.js +++ b/editors/vscode/src/format.js @@ -14,9 +14,10 @@ const COMMENT_COLUMN = 30; // comments move this far past the longest one. const COMMENT_GAP = 4; -// 'l', 'w' or 'c', optionally followed by an argument. The argument keeps its -// own spacing: a condition is free text and a workflow name may contain spaces. -const STEP = /^([lwc])[ \t]+(.*)$/; +// 'l', 'w', 'c' or 'x', optionally followed by an argument. The argument keeps +// its own spacing: a condition is free text, a workflow name may contain +// spaces, and a command's spacing may sit inside its quotes. +const STEP = /^([lwcx])[ \t]+(.*)$/; // A step with no argument at all: a bare 'l' or 'c'. const BARE_STEP = /^[lwc]$/; @@ -58,9 +59,9 @@ function parseLine(raw) { /** * Comment column for one block. * - * Only 'l' and 'w' steps are measured. A 'c' condition is free text that - * routinely runs past any sensible column, and letting it set one would drag - * every comment in the block off to the right. + * Only 'l' and 'w' steps are measured. A 'c' condition is free text, and an + * 'x' command line, that routinely runs past any sensible column, and letting + * either set one would drag every comment in the block off to the right. */ function commentColumn(block) { const widths = block diff --git a/editors/vscode/syntaxes/autoland-plan.tmLanguage.json b/editors/vscode/syntaxes/autoland-plan.tmLanguage.json index 9d3f950..6fb7251 100644 --- a/editors/vscode/syntaxes/autoland-plan.tmLanguage.json +++ b/editors/vscode/syntaxes/autoland-plan.tmLanguage.json @@ -30,6 +30,12 @@ { "include": "#confirm-bare" }, + { + "include": "#exec" + }, + { + "include": "#exec-missing" + }, { "include": "#unknown-step" }, @@ -145,6 +151,27 @@ } } }, + "exec": { + "comment": "x — a command line, run without a shell. Like every step it ends at the first ' #', quotes or not, so it cannot start with '#'.", + "match": "^[ \\t]*(x)[ \\t]+([^ \\t#](?:(?! +#).)*)", + "captures": { + "1": { + "name": "keyword.control.exec.autoland-plan" + }, + "2": { + "name": "string.unquoted.command.autoland-plan" + } + } + }, + "exec-missing": { + "comment": "'x' requires a command.", + "match": "^[ \\t]*(x)(?=[ \\t]*$| +#)", + "captures": { + "1": { + "name": "invalid.illegal.missing-command.autoland-plan" + } + } + }, "unknown-step": { "comment": "Anything else makes the parser fail with 'unrecognized step'.", "match": "^[ \\t]*(\\S(?:(?! +#).)*)", diff --git a/editors/vscode/tests/format.test.js b/editors/vscode/tests/format.test.js index d568bb7..daad640 100644 --- a/editors/vscode/tests/format.test.js +++ b/editors/vscode/tests/format.test.js @@ -33,6 +33,19 @@ test("a c step never sets the column", () => { ); }); +test("an x step never sets the column", () => { + // Like a condition, a command line routinely runs long. + const src = ["x ./scripts/provision.sh --env staging --verbose # provision", "l 101 # land it"].join("\n"); + assert.strictEqual( + formatPlan(src), + ["x ./scripts/provision.sh --env staging --verbose # provision", "l 101 # land it", ""].join("\n") + ); +}); + +test("collapses the space after x but not inside its command", () => { + assert.strictEqual(formatPlan("x bash -c 'echo hi' # say hi\n"), "x bash -c 'echo hi' # say hi\n"); +}); + test("a c step past the column keeps a space before its comment", () => { // Without the space the '#' would become part of the condition, which would // change what the plan does. diff --git a/editors/vscode/tests/steps.autoland-plan b/editors/vscode/tests/steps.autoland-plan index 132652d..a59a82a 100644 --- a/editors/vscode/tests/steps.autoland-plan +++ b/editors/vscode/tests/steps.autoland-plan @@ -72,3 +72,18 @@ l 101 # a tab does not open a comment #<- keyword.control.land.autoland-plan # ^^^^^ - comment.line.number-sign.autoland-plan # ^^^ invalid.illegal.pr-reference.autoland-plan + +x ./scripts/provision.sh --env staging +#<- keyword.control.exec.autoland-plan +# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ string.unquoted.command.autoland-plan + +x bash -c 'make check' # ' #' ends the command, as for every step +#<- keyword.control.exec.autoland-plan +# ^^^^^^^^^^^^^^^^^^^^ string.unquoted.command.autoland-plan +# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ comment.line.number-sign.autoland-plan + +x +#<- invalid.illegal.missing-command.autoland-plan + +x # nothing to run +#<- invalid.illegal.missing-command.autoland-plan diff --git a/src/stack_pr/autoland/commands.py b/src/stack_pr/autoland/commands.py index 48036d3..c6779da 100644 --- a/src/stack_pr/autoland/commands.py +++ b/src/stack_pr/autoland/commands.py @@ -113,7 +113,7 @@ def register_parser( "-i", "--interactive", action="store_true", - help="Edit the landing plan in $EDITOR (add workflow/confirm checkpoints).", + help="Edit the landing plan in $EDITOR (add workflow/confirm/command steps).", ) plan_source.add_argument( "--plan-file", diff --git a/src/stack_pr/autoland/display.py b/src/stack_pr/autoland/display.py index 84a3e1d..56124a8 100644 --- a/src/stack_pr/autoland/display.py +++ b/src/stack_pr/autoland/display.py @@ -109,6 +109,7 @@ def _icon(outcome: StepOutcome) -> str: StepTone.AWAITING_USER: "yellow", StepTone.IN_QUEUE: "cyan", StepTone.AWAITING_WORKFLOW: "blue", + StepTone.RUNNING: "blue", } diff --git a/src/stack_pr/autoland/engine.py b/src/stack_pr/autoland/engine.py index 7c7df73..d7aa648 100644 --- a/src/stack_pr/autoland/engine.py +++ b/src/stack_pr/autoland/engine.py @@ -69,6 +69,7 @@ def execute_step(index: int) -> StepResult: upcoming=functools.partial(upcoming, index), landed_sha=landed_sha, ends_landed_prefix=index == prefix_end, + branch=checkpointer.branch, ) ) _record_landed_sha(ctx, result.landed_sha) diff --git a/src/stack_pr/autoland/model.py b/src/stack_pr/autoland/model.py index 1d1ed18..bcf89d9 100644 --- a/src/stack_pr/autoland/model.py +++ b/src/stack_pr/autoland/model.py @@ -8,6 +8,7 @@ from stack_pr.autoland.steps import ( AutolandStep, ConfirmStep, + ExecStep, LandStep, WorkflowStep, step_from_dict, @@ -16,6 +17,7 @@ __all__ = [ "AutolandStep", "ConfirmStep", + "ExecStep", "LandStep", "LandingContext", "PRState", diff --git a/src/stack_pr/autoland/optimize/stack_merge.py b/src/stack_pr/autoland/optimize/stack_merge.py index 6c985a9..a92a165 100644 --- a/src/stack_pr/autoland/optimize/stack_merge.py +++ b/src/stack_pr/autoland/optimize/stack_merge.py @@ -39,7 +39,7 @@ def stack_merge_run(run: LandingContext, indices: Iterable[int]) -> list[StackEn """The PRs a stack merge starting at the first of *indices* would land. That is the run of consecutive land steps, from the first of *indices*, - whose PRs are still open. Any other step ends the run: a ``w`` or ``c`` + whose PRs are still open. Any other step ends the run: a ``w``, ``c``, or ``x`` step has to see the PRs above it land separately from the ones below it. A run of fewer than two PRs gains nothing from a stack merge, so it comes back empty. diff --git a/src/stack_pr/autoland/plan.py b/src/stack_pr/autoland/plan.py index b55a2aa..2506d39 100644 --- a/src/stack_pr/autoland/plan.py +++ b/src/stack_pr/autoland/plan.py @@ -61,6 +61,8 @@ def format_plan_for_editor(stack: list[StackEntry], plan: list[PlanStep]) -> str "# c [] = pause for manual confirmation; the optional", "# condition names what to verify before proceeding", "# (e.g. 'c QA sign-off complete')", + "# x = run a command, and stop unless it exits 0; it runs", + "# without a shell (use 'x bash -c ...' for pipes)", "#", "# Lines starting with # are comments and are ignored.", "# Blank lines are ignored.", @@ -148,7 +150,7 @@ def landed_prefix_end(plan: list[PlanStep]) -> int: """Index of the last ``l`` step whose PR had already landed, or -1 for none. Every step at or before this index is complete: the PR merged, and the - workflow/confirm steps ahead of it must have run before it could. This is + workflow/confirm/command steps ahead of it must have run before it could. This is the boundary ``execute_plan`` resumes from and the display points at. Prefer ``is_assumed_completed`` for "is this step inside the prefix?" — the @@ -189,14 +191,15 @@ def _checkpoint_keys( def carry_over_progress( old: LandingContext, plan: list[PlanStep], stack: list[StackEntry] ) -> list[int]: - """Credit *plan*'s workflow and confirm steps with results from *old*. + """Credit *plan*'s workflow, confirm, and command steps with results from + *old*. *plan* is a freshly parsed plan for the current *stack*. Land steps need no help: whether a PR merged is GitHub's to say, and parsing already asked. - A workflow or confirm step is credited when *old* completed a step with the - same kind, text, and set of PRs landed before it — so adding, removing, or - reordering steps keeps credit, while changing a step's text, or landing a - different set of PRs ahead of it, makes it run again. + A workflow, confirm, or command step is credited when *old* completed a + step with the same kind, text, and set of PRs landed before it — so adding, + removing, or reordering steps keeps credit, while changing a step's text, + or landing a different set of PRs ahead of it, makes it run again. Returns the indices (into ``old.plan``) of completed checkpoints that found no match in *plan*, so the caller can show what does not carry over. @@ -228,9 +231,9 @@ def plan_from_file(path: Path, stack: list[StackEntry]) -> list[PlanStep]: The file uses the exact same format as the interactive editor (see ``format_plan_for_editor``): ``l [pr]`` / ``w `` / - ``c [condition]`` steps, with ``#`` comments and blank lines ignored. It is - parsed by the same ``parse_plan`` the editor uses, so a file saved from - ``-i`` (or written by hand in that format) round-trips. + ``c [condition]`` / ``x `` steps, with ``#`` comments and blank + lines ignored. It is parsed by the same ``parse_plan`` the editor uses, so + a file saved from ``-i`` (or written by hand in that format) round-trips. """ try: text = path.read_text() diff --git a/src/stack_pr/autoland/repo.py b/src/stack_pr/autoland/repo.py index 30f5cab..17fe504 100644 --- a/src/stack_pr/autoland/repo.py +++ b/src/stack_pr/autoland/repo.py @@ -1,4 +1,5 @@ -"""The local clone: fetching, ancestry, landed SHAs, and rebase + resubmit. +"""The local clone: fetching, ancestry, landed SHAs, rebase + resubmit, and +where it is. ``GitRepo`` is the production ``services.Repo``. It runs git in the current working directory (autoland chdirs into a temporary worktree when ``--branch`` @@ -9,6 +10,7 @@ import contextlib from dataclasses import replace +from pathlib import Path from typing import TYPE_CHECKING from stack_pr import cli @@ -112,3 +114,8 @@ def rebase_and_resubmit(self, common: cli.CommonArgs) -> None: keep_title=True, draft_bitmask=None, ) + + def root(self) -> Path: + """The top-level directory of the checkout autoland runs in.""" + result = runtime.run(["git", "rev-parse", "--show-toplevel"], quiet=True) + return Path(result.stdout.strip()) diff --git a/src/stack_pr/autoland/services.py b/src/stack_pr/autoland/services.py index 6709954..ff2a1cb 100644 --- a/src/stack_pr/autoland/services.py +++ b/src/stack_pr/autoland/services.py @@ -1,13 +1,14 @@ """The tools a plan step may use while it executes, injected through ``Services``. A step, an execution unit, and everything they call while landing (the wait -loops, the stack merge, the landed-SHA lookup, rebase + resubmit) reach the -console, GitHub, the clock, and the local repo only through one ``Services``, -never through module globals, so any of them can be run against fakes. +loops, the stack merge, the landed-SHA lookup, rebase + resubmit, an ``x`` +step's command) reach the console, GitHub, the clock, the local repo, and +subprocesses only through one ``Services``, never through module globals, so +any of them can be run against fakes. ``Services.production()`` binds the real implementations: the ``gh.github`` client, the ``runtime.console``, ``runtime.resilient_sleep``, a ``GitRepo``, -and the polling waits of ``autoland.waits``. +``SubprocessCommands``, and the polling waits of ``autoland.waits``. Module singletons remain only for what runs outside a plan's execution: the command flows' own output and prompts (``commands``), ``--status``, plan @@ -18,10 +19,14 @@ from __future__ import annotations -from collections.abc import Callable +import os +import sys +from collections.abc import Callable, Mapping from dataclasses import dataclass +from pathlib import Path from typing import TYPE_CHECKING, Any, Protocol +from stack_pr import shell_commands from stack_pr.autoland import display, gh, runtime, waits from stack_pr.autoland.repo import GitRepo @@ -100,6 +105,26 @@ def rebase_and_resubmit(self, common: cli.CommonArgs) -> None: """Rebase the local stack onto the target and re-submit it. Raises on failure.""" + def root(self) -> Path: + """The top-level directory of the checkout being landed (the temporary + worktree, with ``--branch``).""" + + +class Commands(Protocol): + """Running a user's command, as an ``x`` step does. + + A service of its own, rather than part of ``Repo``, because what it runs + is the user's program, not an operation on the clone; and so a step can + be tested against commands that just exit with a given status. + """ + + def run(self, argv: list[str], *, cwd: Path, env: Mapping[str, str]) -> int: + """Run *argv* (no shell) in *cwd*, with *env* set on top of autoland's + own environment, its output going straight to the terminal. Returns + its exit status: negative if a signal killed it. Raises OSError if it + can't be started (e.g. no such program). Ctrl+C raises + KeyboardInterrupt out of it once the command has been stopped.""" + @dataclass(frozen=True) class Services: @@ -117,6 +142,7 @@ class Services: # machine spent suspended. sleep: Callable[[int], float] repo: Repo + commands: Commands waits: Waits = None # type: ignore[assignment] # set in __post_init__ def __post_init__(self) -> None: @@ -127,13 +153,15 @@ def __post_init__(self) -> None: @classmethod def production(cls) -> Services: - """The real console, GitHub client, clock, repo, and wait loops.""" + """The real console, GitHub client, clock, repo, commands, and wait + loops.""" ui = ConsoleUI(runtime.console) return cls( github=gh.github, ui=ui, sleep=runtime.resilient_sleep, repo=GitRepo(gh.github, ui), + commands=SubprocessCommands(), ) @@ -159,6 +187,22 @@ def escape(self, text: str) -> str: return display._escape_markup(text) +class SubprocessCommands: + """``Commands`` as real subprocesses.""" + + def run(self, argv: list[str], *, cwd: Path, env: Mapping[str, str]) -> int: + # Whatever autoland has printed must reach the terminal before the + # command's own output, which bypasses this process's buffers. + sys.stdout.flush() + sys.stderr.flush() + # subprocess.run (under run_shell_command) kills the command if + # anything, such as Ctrl+C or a takeover's SIGINT, interrupts the wait + # for it, so no command outlives the run that started it. + return shell_commands.run_shell_command( + argv, quiet=False, check=False, cwd=cwd, env={**os.environ, **env} + ).returncode + + class PollingWaits: """The waits of ``autoland.waits``, polling through *services*.""" diff --git a/src/stack_pr/autoland/steps/__init__.py b/src/stack_pr/autoland/steps/__init__.py index 296ea2f..2b4b6cf 100644 --- a/src/stack_pr/autoland/steps/__init__.py +++ b/src/stack_pr/autoland/steps/__init__.py @@ -16,6 +16,7 @@ step_from_dict, ) from stack_pr.autoland.steps.confirm import ConfirmStep +from stack_pr.autoland.steps.exec import ExecStep from stack_pr.autoland.steps.execution import StepContext, StepResult from stack_pr.autoland.steps.land import LandStep from stack_pr.autoland.steps.view import StepOutcome, StepTone, StepView @@ -25,6 +26,7 @@ "REGISTRY", "AutolandStep", "ConfirmStep", + "ExecStep", "LandStep", "PlanParser", "ReplanKey", diff --git a/src/stack_pr/autoland/steps/base.py b/src/stack_pr/autoland/steps/base.py index fe403fb..ba54a48 100644 --- a/src/stack_pr/autoland/steps/base.py +++ b/src/stack_pr/autoland/steps/base.py @@ -22,7 +22,7 @@ from stack_pr.autoland.steps.execution import StepContext, StepResult from stack_pr.autoland.steps.view import StepView -# A workflow or confirm step's identity for replanning: its keyword, its text, +# A workflow, confirm, or command step's identity for replanning: its keyword, its text, # and the PRs landed before it. A step means "once these PRs have landed, this # holds", so a result recorded under the same PRs is still true in a new plan. ReplanKey = tuple[str, str, frozenset[int]] diff --git a/src/stack_pr/autoland/steps/exec.py b/src/stack_pr/autoland/steps/exec.py new file mode 100644 index 0000000..3f0443e --- /dev/null +++ b/src/stack_pr/autoland/steps/exec.py @@ -0,0 +1,162 @@ +"""``x ``: run a command, and stop the land unless it succeeds.""" + +from __future__ import annotations + +import shlex +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from stack_pr.autoland.steps.base import ( + AutolandStep, + PlanParser, + ReplanKey, + register_step, +) +from stack_pr.autoland.steps.execution import StepContext, StepResult +from stack_pr.autoland.steps.view import StepOutcome, StepTone, StepView + +if TYPE_CHECKING: + from stack_pr.autoland.model import StackEntry + + +# As for WorkflowStep, "skipped" is set only by assume_completed: the command +# sits inside the plan's already-landed prefix, so it must have run before the +# PRs after it could merge. +_STATE_STATUS = { + "pending": (StepOutcome.PENDING, "Pending", StepTone.MUTED), + "running": (StepOutcome.ACTIVE, "Running", StepTone.RUNNING), + "succeeded": (StepOutcome.DONE, "Succeeded", StepTone.SUCCESS), + "failed": (StepOutcome.FAILED, "Failed", StepTone.FAILURE), + "skipped": (StepOutcome.DONE, "Assumed completed", StepTone.SUCCESS), +} + + +def _split(command: str) -> list[str]: + """*command* as an argument list, split the way a POSIX shell would split + it (quotes, backslashes), but with no expansion: there is no shell.""" + return shlex.split(command) + + +@register_step +@dataclass +class ExecStep(AutolandStep): + """Run a command once the PRs before it have landed; it must exit 0. + + ``command`` is the text after ``x``, as written in the plan. It runs with + no shell, from the top of the checkout being landed, with the run's + facts in ``AUTOLAND_*`` environment variables (see ``execute``). Any + other exit status stops the run. A command that failed, or was + interrupted, runs again on ``--resume``, so it should be safe to re-run. + """ + + kind = "exec" + keyword = "x" + label = "Execute" + # A bare 'x' names no command; it is not an 'x' step at all. + bare_keyword_ok = False + # A command line routinely runs long; letting it decide the comment column + # would drag every comment off to the right. + aligns_comments = False + + command: str + state: str = "pending" # pending, running, succeeded, failed, skipped + error_message: str = "" + + @classmethod + def parse(cls, args: str, parser: PlanParser) -> ExecStep: + if not args: + raise ValueError(f"Line {parser.line_num}: 'x' requires a command") + try: + _split(args) + except ValueError as e: + raise ValueError( + f"Line {parser.line_num}: malformed command {args!r}: {e}" + ) from None + return cls(command=args) + + def plan_line(self, stack: list[StackEntry]) -> tuple[str, str]: + return f"x {self.command}", "" + + def assume_completed(self) -> None: + self.state = "skipped" + + def is_checkpoint_done(self) -> bool: + return self.state in ("succeeded", "skipped") + + def replan_key(self, landed_prs: frozenset[int]) -> ReplanKey | None: + return (self.keyword, self.command, landed_prs) + + def credit_from(self, old: AutolandStep) -> None: + if isinstance(old, ExecStep): + self.state = old.state + self.error_message = "" + + def view( + self, stack: list[StackEntry], *, is_next: bool, assumed_completed: bool + ) -> StepView: + outcome, status, tone = _STATE_STATUS.get( + self.state, (StepOutcome.PENDING, "Pending", StepTone.MUTED) + ) + return StepView( + outcome=outcome, + status=status, + tone=tone, + key=self.command, + description=f"Run {self.command}", + detail=(self.error_message,) if self.error_message else (), + data={"command": self.command}, + ) + + def execute(self, sc: StepContext) -> StepResult: + """Run the command, with the run's facts in its environment: + + - ``AUTOLAND_LANDED_SHA``: the commit of the code landed so far (see + ``StepContext.landed_sha``), or "" if it can't be found; + - ``AUTOLAND_LANDED_PRS``: the PRs the plan has landed before this + step, space-separated in plan order, including those that landed in + an earlier run; + - ``AUTOLAND_BRANCH``, ``AUTOLAND_REMOTE``, ``AUTOLAND_TARGET``: the + branch whose stack is landing, and the remote and branch it lands + into. + """ + # "succeeded" before the run reaches it means --replan carried the + # result over from the run this one replaced. + if self.state in ("skipped", "succeeded"): + return StepResult.done() + ui = sc.services.ui + ui.print(sc.banner(f"Run {ui.escape(self.command)}")) + stack = sc.run.stack + landed = [ + pr + for step in sc.run.plan[: sc.index] + if (pr := step.landed_pr(stack)) is not None + ] + env = { + "AUTOLAND_LANDED_SHA": sc.landed_sha(), + "AUTOLAND_LANDED_PRS": " ".join(str(pr) for pr in landed), + "AUTOLAND_BRANCH": sc.branch, + "AUTOLAND_REMOTE": sc.common.remote, + "AUTOLAND_TARGET": sc.common.target, + } + self.state, self.error_message = "running", "" + # So that --status shows the command running, and an interrupted run + # resumes by running it again. + sc.checkpoint() + try: + status = sc.services.commands.run( + _split(self.command), cwd=sc.services.repo.root(), env=env + ) + except OSError as e: + return self._fail(f"could not start: {e.strerror or e}") + if status < 0: + return self._fail(f"killed by signal {-status}") + if status: + return self._fail(f"exited with status {status}") + self.state = "succeeded" + ui.print("[green]Command succeeded[/green]") + return StepResult.done() + + def _fail(self, why: str) -> StepResult: + self.state = "failed" + self.error_message = f"Command {why}" + return StepResult.abort(f"Command `{self.command}` {why}") diff --git a/src/stack_pr/autoland/steps/execution.py b/src/stack_pr/autoland/steps/execution.py index b4e3630..0e94f86 100644 --- a/src/stack_pr/autoland/steps/execution.py +++ b/src/stack_pr/autoland/steps/execution.py @@ -73,6 +73,9 @@ class StepContext: # Whether this step is the last of the plan's already-landed prefix (see # ``plan.landed_prefix_end``), the one whose landed code later steps follow. ends_landed_prefix: bool = False + # The local branch whose stack is being landed (the run's state file is + # this branch's), or "" if unknown. + branch: str = "" def banner(self, title: str, style: str = "bold") -> str: """The markup heading this step's output: "Step i/n: ".""" diff --git a/src/stack_pr/autoland/steps/view.py b/src/stack_pr/autoland/steps/view.py index 96aff81..5d7490e 100644 --- a/src/stack_pr/autoland/steps/view.py +++ b/src/stack_pr/autoland/steps/view.py @@ -43,6 +43,7 @@ class StepTone(Enum): AWAITING_USER = "awaiting_user" IN_QUEUE = "in_queue" AWAITING_WORKFLOW = "awaiting_workflow" + RUNNING = "running" # running a command of its own @dataclass(frozen=True) diff --git a/tests/autoland_fakes.py b/tests/autoland_fakes.py index 58536df..5b46bc8 100644 --- a/tests/autoland_fakes.py +++ b/tests/autoland_fakes.py @@ -9,6 +9,8 @@ - ``FakeWaits``: waits that succeed or fail at once, per outcome function. - ``FakeRepo``: landed SHAs from a table; counts rebases and fetches; knows the ancestry it is given, plus what it learns from a fetch. +- ``FakeCommands``: records each command run; exits with a scripted status, + or raises a scripted error. - ``FakeGitHub``: an in-memory GitHub (PR states, reviews, checks, the merge queue, workflow runs, GitHub stacks), whose calls can be scripted. - ``FakeSleep``: records each sleep; can end a wait by raising @@ -21,9 +23,10 @@ from __future__ import annotations import functools -from collections.abc import Callable +from collections.abc import Callable, Mapping from dataclasses import dataclass, field from functools import cached_property +from pathlib import Path from typing import Any, TypeVar, cast from stack_pr import cli @@ -191,6 +194,7 @@ class FakeRepo: lookups: list[int | None] = field(default_factory=list) rebases: int = 0 fetches: int = 0 + top_level: Path = Path("/work/repo") def fetch_target(self, common: cli.CommonArgs) -> None: self.fetches += 1 @@ -208,6 +212,37 @@ def rebase_and_resubmit(self, common: cli.CommonArgs) -> None: if self.rebase_error is not None: raise self.rebase_error + def root(self) -> Path: + return self.top_level + + +# --------------------------------------------------------------------------- +# Commands +# --------------------------------------------------------------------------- + + +@dataclass +class CommandRun: + argv: list[str] + cwd: Path + env: dict[str, str] + + +@dataclass +class FakeCommands: + """Each command exits with the next of *results* (0 once they run out); + a result that is an exception is raised instead.""" + + results: list[int | BaseException] = field(default_factory=list) + runs: list[CommandRun] = field(default_factory=list) + + def run(self, argv: list[str], *, cwd: Path, env: Mapping[str, str]) -> int: + self.runs.append(CommandRun(list(argv), cwd, dict(env))) + result = self.results.pop(0) if self.results else 0 + if isinstance(result, BaseException): + raise result + return result + # --------------------------------------------------------------------------- # GitHub @@ -467,6 +502,7 @@ class FakeServices: ui: FakeUI = field(default_factory=FakeUI) sleep: Callable[[int], float] = field(default_factory=FakeSleep) repo: FakeRepo = field(default_factory=FakeRepo) + commands: FakeCommands = field(default_factory=FakeCommands) waits: FakeWaits | None = field(default_factory=FakeWaits) @classmethod @@ -481,5 +517,6 @@ def services(self) -> Services: ui=self.ui, sleep=self.sleep, repo=self.repo, + commands=self.commands, waits=cast("Any", self.waits), ) diff --git a/tests/autoland_scenarios.py b/tests/autoland_scenarios.py index 266db2b..eb51468 100644 --- a/tests/autoland_scenarios.py +++ b/tests/autoland_scenarios.py @@ -16,9 +16,10 @@ - the console (``runtime.console``) is the plain-text console, with rich off, the Unicode glyphs pinned, and ``input()`` scripted; - ``runtime.resilient_sleep`` doesn't sleep, but advances ``FakeWorld``'s clock; -- subprocesses: ``shell_commands.run_shell_command`` (under ``runtime.run`` - and the ``gh`` JSON helpers) and ``cli.get_command_output`` are answered by - ``FakeWorld``, which plays both GitHub (``gh``) and the local clone (``git``); +- subprocesses: ``shell_commands.run_shell_command`` (under ``runtime.run``, + the ``gh`` JSON helpers, and an ``x`` step's command) and + ``cli.get_command_output`` are answered by ``FakeWorld``, which plays GitHub + (``gh``), the local clone (``git``), and the scenario's own commands; - stack-pr itself: ``cli.get_stack`` returns the scenario's stack, ``cli.command_submit`` (the resubmit after a merge) does nothing, and ``git.get_current_branch_name`` is pinned; @@ -33,6 +34,7 @@ import hashlib import json import re +import shlex import subprocess from collections.abc import Callable from dataclasses import dataclass, field @@ -52,6 +54,7 @@ from tests.helpers import PR_URL, common_args, mock_entry BRANCH = "feat" +REPO_ROOT = "/work/repo" # the top of the clone, as git reports it OWNER, REPO = "o", "r" WORKFLOW_RUN_ID = 9000 # + the PR number: the CI run a PR's checks belong to @@ -122,6 +125,8 @@ class FakeWorld: stack_merge: str = "merge" stack_merge_lands: list[int] = field(default_factory=list) stack_merge_message: str = "" + # An ``x`` step's command line -> the status it exits with. + commands: dict[str, int] = field(default_factory=dict) main_head: str = field(default_factory=lambda: sha("main-0")) stacks: dict[int, list[int]] = field(default_factory=dict) @@ -167,6 +172,8 @@ def _git(self, args: list[str]) -> str: return "" if args == ["rev-parse", "origin/main"]: return self.main_head + "\n" + if args == ["rev-parse", "--show-toplevel"]: + return REPO_ROOT + "\n" if args[:3] == ["rev-parse", "--verify", "--quiet"]: number = int(args[3].rsplit("/", 1)[-1]) return commit_sha(number) + "\n" @@ -201,6 +208,18 @@ def _gh(self, args: list[str], stdin: str | None) -> str: # noqa: PLR0911 return "" if out is None else json.dumps(out) raise AssertionError(f"unexpected gh command: {args}") + def run_command(self, cmd: list[str], cwd: str, env: dict[str, str]) -> int: + """Run one of the scenario's commands: show what it was run with, as its + output would show on the terminal, and exit with its status.""" + line = shlex.join(cmd) + if line not in self.commands: + raise AssertionError(f"unexpected command: {cmd}") + print(f"({line} in {cwd})") + for name in sorted(env): + if name.startswith("AUTOLAND_"): + print(f" {name}={env[name]}") + return self.commands[line] + def _checks(self, pr: FakePR) -> list[dict[str, str]]: bucket = pr.checks.pop(0) if len(pr.checks) > 1 else pr.checks[0] state = {"pass": "SUCCESS", "fail": "FAILURE", "pending": "IN_PROGRESS"} @@ -312,6 +331,8 @@ class Scenario: stack_merge: str = "merge" stack_merge_lands: list[int] = field(default_factory=list) stack_merge_message: str = "" + # An ``x`` step's command line -> the status it exits with. + commands: dict[str, int] = field(default_factory=dict) # PRs that landed before this run (and so are no longer in the stack). landed: list[int] = field(default_factory=list) # Builds the checkpoint of an earlier run, which this run resumes or @@ -401,9 +422,13 @@ def sleep(_seconds: int) -> float: mocker.patch.object(runtime, "resilient_sleep", side_effect=sleep) - # Subprocesses: git and gh go to the world; anything else is a test bug. + # Subprocesses: git, gh, and the scenario's commands go to the world; + # anything else is a test bug. def run_shell_command(cmd: Any, **kwargs: Any) -> subprocess.CompletedProcess: args = [str(c) for c in cmd] + if args[0] not in ("git", "gh"): + status = world.run_command(args, str(kwargs["cwd"]), kwargs["env"]) + return subprocess.CompletedProcess(args, status) try: out = world(args, kwargs.get("input")) except subprocess.CalledProcessError as e: @@ -462,6 +487,7 @@ def run_scenario( stack_merge=scenario.stack_merge, stack_merge_lands=scenario.stack_merge_lands, stack_merge_message=scenario.stack_merge_message, + commands=dict(scenario.commands), ) for number in scenario.landed: world.merge(world.prs[number]) diff --git a/tests/golden/autoland/checks_fail.plan.txt b/tests/golden/autoland/checks_fail.plan.txt index da890ac..8e9ad6a 100644 --- a/tests/golden/autoland/checks_fail.plan.txt +++ b/tests/golden/autoland/checks_fail.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/confirm_eof.plan.txt b/tests/golden/autoland/confirm_eof.plan.txt index 460ebb2..52154fd 100644 --- a/tests/golden/autoland/confirm_eof.plan.txt +++ b/tests/golden/autoland/confirm_eof.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/exec_steps.checkpoint.json b/tests/golden/autoland/exec_steps.checkpoint.json new file mode 100644 index 0000000..a56de2d --- /dev/null +++ b/tests/golden/autoland/exec_steps.checkpoint.json @@ -0,0 +1,73 @@ +{ + "version": 1, + "branch": "feat", + "base": "main", + "plan_file": "<TMP>/plan.autoland-plan", + "current_step": 3, + "last_landed_sha": "9cc0a8c62f2d99c85284a96fe68191bd3843f14b", + "abort_reason": "Command `./scripts/smoke-test.sh` exited with status 3", + "stack": [ + { + "pr_url": "https://github.com/o/r/pull/101", + "pr_number": 101, + "branch": "TestBot/stack/101", + "title": "Add the widget model", + "review_decision": "APPROVED", + "state": "merged", + "check_retries": 0, + "queue_retries": 0, + "error_message": "" + }, + { + "pr_url": "https://github.com/o/r/pull/102", + "pr_number": 102, + "branch": "TestBot/stack/102", + "title": "Expose widgets over the API", + "review_decision": "APPROVED", + "state": "merged", + "check_retries": 0, + "queue_retries": 0, + "error_message": "" + }, + { + "pr_url": "https://github.com/o/r/pull/103", + "pr_number": 103, + "branch": "TestBot/stack/103", + "title": "Show widgets in the dashboard", + "review_decision": "APPROVED", + "state": "pending", + "check_retries": 0, + "queue_retries": 0, + "error_message": "" + } + ], + "plan": [ + { + "type": "land", + "entry_index": 0, + "pr_number": 101 + }, + { + "type": "exec", + "command": "./scripts/provision.sh --env 'staging east'", + "state": "succeeded", + "error_message": "" + }, + { + "type": "land", + "entry_index": 1, + "pr_number": 102 + }, + { + "type": "exec", + "command": "./scripts/smoke-test.sh", + "state": "failed", + "error_message": "Command exited with status 3" + }, + { + "type": "land", + "entry_index": 2, + "pr_number": 103 + } + ] +} diff --git a/tests/golden/autoland/exec_steps.out.txt b/tests/golden/autoland/exec_steps.out.txt new file mode 100644 index 0000000..9734177 --- /dev/null +++ b/tests/golden/autoland/exec_steps.out.txt @@ -0,0 +1,84 @@ + +Discovering stack... + + +Autoland plan · 5 steps · 0 done · 5 remaining + +→ ⬜ 1. Pending | Land | #101 - Add the widget model + ⬜ 2. Pending | Execute | ./scripts/provision.sh --env 'staging east' + ⬜ 3. Pending | Land | #102 - Expose widgets over the API + ⬜ 4. Pending | Execute | ./scripts/smoke-test.sh + ⬜ 5. Pending | Land | #103 - Show widgets in the dashboard +State file: <TMP>/home/.stack-pr/autoland/feat.json + + +============================================================ +Step 1/5: Landing PR #101 — Add the widget model +============================================================ +PR #101 is mergeable (mergeStateStatus=CLEAN) + +Adding PR #101 to merge queue +$ gh pr merge 101 --squash +PR #101: in merge queue (0m) — polling in 120s + +PR #101 merged! + +Rebasing stack on origin/main... +$ git fetch origin main +$ git rebase origin/main +Re-submitting stack... + +============================================================ +Step 2/5: Run ./scripts/provision.sh --env 'staging east' +============================================================ +(./scripts/provision.sh --env 'staging east' in /work/repo) + AUTOLAND_BRANCH=feat + AUTOLAND_LANDED_PRS=101 + AUTOLAND_LANDED_SHA=78e4ec1063c72debbb00f2ff966f72377789cdae + AUTOLAND_REMOTE=origin + AUTOLAND_TARGET=main +Command succeeded + +============================================================ +Step 3/5: Landing PR #102 — Expose widgets over the API +============================================================ +PR #102 is mergeable (mergeStateStatus=CLEAN) + +Adding PR #102 to merge queue +$ gh pr merge 102 --squash +PR #102: in merge queue (0m) — polling in 120s + +PR #102 merged! + +Rebasing stack on origin/main... +$ git fetch origin main +$ git rebase origin/main +Re-submitting stack... + +============================================================ +Step 4/5: Run ./scripts/smoke-test.sh +============================================================ +(./scripts/smoke-test.sh in /work/repo) + AUTOLAND_BRANCH=feat + AUTOLAND_LANDED_PRS=101 102 + AUTOLAND_LANDED_SHA=9cc0a8c62f2d99c85284a96fe68191bd3843f14b + AUTOLAND_REMOTE=origin + AUTOLAND_TARGET=main + + + +Autoland plan · 5 steps · 3 done · 2 remaining + + ✅ 1. Merged | Land | #101 - Add the widget model + ✅ 2. Succeeded | Execute | ./scripts/provision.sh --env 'staging east' + ✅ 3. Merged | Land | #102 - Expose widgets over the API +→ ❌ 4. Failed | Execute | ./scripts/smoke-test.sh + ↳ Command exited with status 3 + ⬜ 5. Pending | Land | #103 - Show widgets in the dashboard + +ABORTED: Command `./scripts/smoke-test.sh` exited with status 3 + +Landing failed: Command `./scripts/smoke-test.sh` exited with status 3 + +State saved to <TMP>/home/.stack-pr/autoland/feat.json — resume with --resume + diff --git a/tests/golden/autoland/exec_steps.plan.txt b/tests/golden/autoland/exec_steps.plan.txt new file mode 100644 index 0000000..784a1e4 --- /dev/null +++ b/tests/golden/autoland/exec_steps.plan.txt @@ -0,0 +1,18 @@ +# Autoland plan — edit steps below. +# l [<pr>] = land that PR (a number, or its URL); a bare 'l' +# lands the next PR in the stack instead +# w <workflow> = wait for a workflow to complete +# c [<condition>] = pause for manual confirmation; the optional +# condition names what to verify before proceeding +# (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) +# +# Lines starting with # are comments and are ignored. +# Blank lines are ignored. +# +l 101 # Add the widget model +x ./scripts/provision.sh --env 'staging east' +l 102 # Expose widgets over the API +x ./scripts/smoke-test.sh +l 103 # Show widgets in the dashboard diff --git a/tests/golden/autoland/exec_steps.status.json b/tests/golden/autoland/exec_steps.status.json new file mode 100644 index 0000000..6e63fd7 --- /dev/null +++ b/tests/golden/autoland/exec_steps.status.json @@ -0,0 +1,71 @@ +{ + "status": "stopped", + "branch": "feat", + "base": "main", + "state_file": "<TMP>/home/.stack-pr/autoland/feat.json", + "state_file_exists": true, + "lock_file": "<TMP>/home/.stack-pr/autoland/feat.json.lock", + "pid": null, + "last_saved": "<SAVED>", + "abort_reason": "Command `./scripts/smoke-test.sh` exited with status 3", + "resume_command": "stack-pr autoland --resume", + "plan": { + "total": 5, + "done": 3, + "remaining": 2, + "steps": [ + { + "number": 1, + "type": "land", + "outcome": "done", + "status": "Merged", + "is_next": false, + "pr_number": 101, + "pr_url": "https://github.com/o/r/pull/101", + "title": "Add the widget model", + "detail": null + }, + { + "number": 2, + "type": "exec", + "outcome": "done", + "status": "Succeeded", + "is_next": false, + "command": "./scripts/provision.sh --env 'staging east'", + "detail": null + }, + { + "number": 3, + "type": "land", + "outcome": "done", + "status": "Merged", + "is_next": false, + "pr_number": 102, + "pr_url": "https://github.com/o/r/pull/102", + "title": "Expose widgets over the API", + "detail": null + }, + { + "number": 4, + "type": "exec", + "outcome": "failed", + "status": "Failed", + "is_next": true, + "command": "./scripts/smoke-test.sh", + "detail": "Command exited with status 3" + }, + { + "number": 5, + "type": "land", + "outcome": "pending", + "status": "Pending", + "is_next": false, + "pr_number": 103, + "pr_url": "https://github.com/o/r/pull/103", + "title": "Show widgets in the dashboard", + "detail": null + } + ] + }, + "other_runs": [] +} diff --git a/tests/golden/autoland/exec_steps.status.txt b/tests/golden/autoland/exec_steps.status.txt new file mode 100644 index 0000000..459f6dc --- /dev/null +++ b/tests/golden/autoland/exec_steps.status.txt @@ -0,0 +1,21 @@ +Autoland status + + Status: Stopped — not running, can be resumed + Branch: feat + Base: main + State file: <TMP>/home/.stack-pr/autoland/feat.json + Last saved: <SAVED> + + +Autoland plan · 5 steps · 3 done · 2 remaining + + ✅ 1. Merged | Land | #101 - Add the widget model + ✅ 2. Succeeded | Execute | ./scripts/provision.sh --env 'staging east' + ✅ 3. Merged | Land | #102 - Expose widgets over the API +→ ❌ 4. Failed | Execute | ./scripts/smoke-test.sh + ↳ Command exited with status 3 + ⬜ 5. Pending | Land | #103 - Show widgets in the dashboard + +ABORTED: Command `./scripts/smoke-test.sh` exited with status 3 + +Resume with: stack-pr autoland --resume diff --git a/tests/golden/autoland/landed_prefix.plan.txt b/tests/golden/autoland/landed_prefix.plan.txt index f22249b..6868655 100644 --- a/tests/golden/autoland/landed_prefix.plan.txt +++ b/tests/golden/autoland/landed_prefix.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/merge_as_stack.plan.txt b/tests/golden/autoland/merge_as_stack.plan.txt index 070b643..4c60fd2 100644 --- a/tests/golden/autoland/merge_as_stack.plan.txt +++ b/tests/golden/autoland/merge_as_stack.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/one_at_a_time.plan.txt b/tests/golden/autoland/one_at_a_time.plan.txt index f77bb05..7cce153 100644 --- a/tests/golden/autoland/one_at_a_time.plan.txt +++ b/tests/golden/autoland/one_at_a_time.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/replan_carry_over.plan.txt b/tests/golden/autoland/replan_carry_over.plan.txt index 96d22eb..5e66782 100644 --- a/tests/golden/autoland/replan_carry_over.plan.txt +++ b/tests/golden/autoland/replan_carry_over.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/resume_mid_plan.plan.txt b/tests/golden/autoland/resume_mid_plan.plan.txt index f22249b..6868655 100644 --- a/tests/golden/autoland/resume_mid_plan.plan.txt +++ b/tests/golden/autoland/resume_mid_plan.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/stack_merge_falls_through.plan.txt b/tests/golden/autoland/stack_merge_falls_through.plan.txt index c2f2a6d..bf6edd2 100644 --- a/tests/golden/autoland/stack_merge_falls_through.plan.txt +++ b/tests/golden/autoland/stack_merge_falls_through.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/golden/autoland/workflow_fails.plan.txt b/tests/golden/autoland/workflow_fails.plan.txt index 258a544..5219d0e 100644 --- a/tests/golden/autoland/workflow_fails.plan.txt +++ b/tests/golden/autoland/workflow_fails.plan.txt @@ -5,6 +5,8 @@ # c [<condition>] = pause for manual confirmation; the optional # condition names what to verify before proceeding # (e.g. 'c QA sign-off complete') +# x <command> = run a command, and stop unless it exits 0; it runs +# without a shell (use 'x bash -c ...' for pipes) # # Lines starting with # are comments and are ignored. # Blank lines are ignored. diff --git a/tests/test_autoland_golden.py b/tests/test_autoland_golden.py index caff3ee..8fa3879 100644 --- a/tests/test_autoland_golden.py +++ b/tests/test_autoland_golden.py @@ -205,6 +205,22 @@ def _one_at_a_time_prs() -> list[FakePR]: ), workflows={"deploy.yaml": "success"}, ), + # Commands between the land steps: the first succeeds, and the second + # fails, which stops the run before the last PR. + Scenario( + name="exec_steps", + prs=_prs(101, 102, 103), + stack=[101, 102, 103], + plan=( + "l\nx ./scripts/provision.sh --env 'staging east'\nl\n" + "x ./scripts/smoke-test.sh # must pass before #103\nl\n" + ), + commands={ + "./scripts/provision.sh --env 'staging east'": 0, + "./scripts/smoke-test.sh": 3, + }, + expect_exit=1, + ), # --replan with an edited plan file: finished checkpoints after the same # PRs carry over, a new one runs, and a removed one is reported. Scenario( diff --git a/tests/test_autoland_step_execution.py b/tests/test_autoland_step_execution.py index 6ef16e3..8850c3f 100644 --- a/tests/test_autoland_step_execution.py +++ b/tests/test_autoland_step_execution.py @@ -7,6 +7,9 @@ from __future__ import annotations +import json +import shlex +import sys from pathlib import Path import pytest @@ -14,17 +17,19 @@ from stack_pr.autoland import engine from stack_pr.autoland.model import ( ConfirmStep, + ExecStep, LandingContext, LandStep, PRState, StackEntry, WorkflowStep, ) -from stack_pr.autoland.services import Services +from stack_pr.autoland.services import Services, SubprocessCommands from stack_pr.autoland.state import AutolandCheckpointer from stack_pr.autoland.steps import AutolandStep, StepContext, StepResult from tests.autoland_fakes import ( OPTS, + FakeCommands, FakeRepo, FakeServices, FakeUI, @@ -42,6 +47,7 @@ def services( ui: FakeUI | None = None, waits: FakeWaits | None = None, repo: FakeRepo | None = None, + commands: FakeCommands | None = None, ) -> Services: """Fake services for a step, which must not call GitHub or sleep itself: it waits only through ``waits``.""" @@ -50,6 +56,7 @@ def services( ui=ui or FakeUI(), sleep=forbidden_sleep, repo=repo or FakeRepo(), + commands=commands or FakeCommands(), waits=waits or FakeWaits(), ).services @@ -276,6 +283,202 @@ def test_confirmed_step_does_not_prompt() -> None: assert ui.lines == [] +# --------------------------------------------------------------------------- +# ExecStep +# --------------------------------------------------------------------------- + + +def run_exec( + step: ExecStep, svc: Services, *, landed_sha: str = "sha102" +) -> StepResult: + """Execute *step* after a plan that landed #100 in an earlier run, then + #101 and #102 (with a confirmation between them).""" + entries = stack(101, 102, 103) + entries[0].state = entries[1].state = PRState.MERGED + plan: list[AutolandStep] = [ + LandStep(entry_index=-1, pr_number=100), + LandStep(entry_index=0), + ConfirmStep(confirmed=True), + LandStep(entry_index=1, pr_number=102), + step, + LandStep(entry_index=2), + ] + return step.execute( + StepContext( + index=4, + total=len(plan), + run=LandingContext(stack=entries, plan=plan), + opts=OPTS, + common=common_args(remote="upstream", target="trunk"), + services=svc, + checkpoint=lambda: None, + upcoming=list, + landed_sha=lambda: landed_sha, + branch="feat", + ) + ) + + +def test_exec_step_runs_its_command_in_the_repo_with_the_run_in_its_env() -> None: + commands, ui = FakeCommands(), FakeUI() + repo = FakeRepo(top_level=Path("/work/worktree/repo")) + step = ExecStep(command="./provision.sh --env 'staging east' $HOME") + + result = run_exec(step, services(ui, repo=repo, commands=commands)) + + assert result == StepResult() + assert step.state == "succeeded" + [run] = commands.runs + # Split like a shell would, but nothing is expanded: there is no shell. + assert run.argv == ["./provision.sh", "--env", "staging east", "$HOME"] + assert run.cwd == Path("/work/worktree/repo") + assert run.env == { + "AUTOLAND_LANDED_SHA": "sha102", + "AUTOLAND_LANDED_PRS": "100 101 102", + "AUTOLAND_BRANCH": "feat", + "AUTOLAND_REMOTE": "upstream", + "AUTOLAND_TARGET": "trunk", + } + assert "Step 5/6: Run ./provision.sh" in ui.output + + +def test_exec_step_shows_it_is_running_in_the_checkpoint() -> None: + states: list[str] = [] + step = ExecStep(command="true") + entries = stack(101) + + step.execute( + StepContext( + index=0, + total=2, + run=LandingContext(stack=entries, plan=[step, LandStep(entry_index=0)]), + opts=OPTS, + common=common_args(), + services=services(), + checkpoint=lambda: states.append(step.state), + upcoming=list, + landed_sha=lambda: "", + ) + ) + + assert states == ["running"] + assert step.state == "succeeded" + + +@pytest.mark.parametrize( + ("result", "reason"), + [ + (3, "Command `./smoke-test.sh` exited with status 3"), + (-9, "Command `./smoke-test.sh` killed by signal 9"), + ( + FileNotFoundError(2, "No such file or directory"), + "Command `./smoke-test.sh` could not start: No such file or directory", + ), + ( + PermissionError(13, "Permission denied"), + "Command `./smoke-test.sh` could not start: Permission denied", + ), + ], +) +def test_exec_step_aborts_unless_its_command_succeeds( + result: int | OSError, reason: str +) -> None: + step = ExecStep(command="./smoke-test.sh") + + outcome = run_exec(step, services(commands=FakeCommands(results=[result]))) + + assert outcome == StepResult(abort_reason=reason) + assert step.state == "failed" + assert step.error_message == reason.replace("Command `./smoke-test.sh`", "Command") + + +@pytest.mark.parametrize("state", ["skipped", "succeeded"]) +def test_exec_step_already_done_does_not_run(state: str) -> None: + commands, ui = FakeCommands(), FakeUI() + + result = run_exec( + ExecStep(command="./provision.sh", state=state), + services(ui, commands=commands), + ) + + assert result == StepResult() + assert commands.runs == [] + assert ui.lines == [] + + +@pytest.mark.parametrize("state", ["failed", "running"]) +def test_exec_step_runs_again_after_failing_or_being_interrupted(state: str) -> None: + commands = FakeCommands() + step = ExecStep(command="./provision.sh", state=state, error_message="boom") + + result = run_exec(step, services(commands=commands)) + + assert result == StepResult() + assert len(commands.runs) == 1 + assert (step.state, step.error_message) == ("succeeded", "") + + +_ENV_SCRIPT = """\ +import json, os, sys +names = [n for n in os.environ if n.startswith("AUTOLAND_")] + ["INHERITED"] +facts = {"cwd": os.getcwd(), "args": sys.argv[1:]} +facts.update({n: os.environ.get(n) for n in names}) +with open(os.environ["OUT"], "w") as f: + json.dump(facts, f) +sys.exit(int(sys.argv[1])) +""" + + +@pytest.mark.parametrize("status", [0, 4]) +def test_exec_step_runs_a_real_command( + status: int, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + script = tmp_path / "script.py" + script.write_text(_ENV_SCRIPT) + out = tmp_path / "facts.json" + monkeypatch.setenv("OUT", str(out)) + monkeypatch.setenv("INHERITED", "yes") + root = tmp_path / "repo" + root.mkdir() + svc = FakeServices( + ui=FakeUI(), + repo=FakeRepo(top_level=root), + commands=SubprocessCommands(), # type: ignore[arg-type] + ).services + step = ExecStep( + command=shlex.join([sys.executable, str(script), str(status), "a b"]) + ) + + result = run_exec(step, svc) + + assert result.aborted == (status != 0) + assert step.state == ("succeeded" if status == 0 else "failed") + assert json.loads(out.read_text()) == { + "cwd": str(root.resolve()), + "args": [str(status), "a b"], + "AUTOLAND_LANDED_SHA": "sha102", + "AUTOLAND_LANDED_PRS": "100 101 102", + "AUTOLAND_BRANCH": "feat", + "AUTOLAND_REMOTE": "upstream", + "AUTOLAND_TARGET": "trunk", + "INHERITED": "yes", + } + + +def test_exec_step_reports_a_missing_program(tmp_path: Path) -> None: + svc = FakeServices( + repo=FakeRepo(top_level=tmp_path), + commands=SubprocessCommands(), # type: ignore[arg-type] + ).services + step = ExecStep(command="./no-such-script.sh") + + result = run_exec(step, svc) + + assert result.abort_reason == ( + "Command `./no-such-script.sh` could not start: No such file or directory" + ) + + # --------------------------------------------------------------------------- # The engine records what steps report # --------------------------------------------------------------------------- diff --git a/tests/test_autoland_steps.py b/tests/test_autoland_steps.py index 1233a55..0533187 100644 --- a/tests/test_autoland_steps.py +++ b/tests/test_autoland_steps.py @@ -12,12 +12,17 @@ import pytest from stack_pr.autoland.model import LandingContext, PRState, StackEntry -from stack_pr.autoland.plan import format_plan_for_editor, parse_plan +from stack_pr.autoland.plan import ( + carry_over_progress, + format_plan_for_editor, + parse_plan, +) from stack_pr.autoland.state import AutolandCheckpointer from stack_pr.autoland.steps import ( REGISTRY, AutolandStep, ConfirmStep, + ExecStep, LandStep, PlanParser, StepContext, @@ -40,6 +45,7 @@ LandStep: LandStep(entry_index=0, pr_number=101), WorkflowStep: WorkflowStep(workflow="deploy.yaml"), ConfirmStep: ConfirmStep(condition="QA sign-off complete"), + ExecStep: ExecStep(command="./provision.sh --env 'staging east'"), } @@ -66,6 +72,7 @@ def test_every_registered_step_type_has_a_view( ("land", "l", LandStep), ("workflow", "w", WorkflowStep), ("confirm", "c", ConfirmStep), + ("exec", "x", ExecStep), ], ) def test_registry_looks_up_step_types( @@ -355,3 +362,108 @@ def test_confirm_view_without_a_condition() -> None: assert view.key == "(no condition)" assert view.description == "Manual confirmation" assert view.data == {"condition": ""} + + +# --------------------------------------------------------------------------- +# ExecStep +# --------------------------------------------------------------------------- + + +def test_exec_step_keeps_its_command_as_written() -> None: + # Spacing and quoting are the command's own; a ' #' still starts a comment. + [_, step] = parse_plan( + "l\nx bash -c 'echo \"$X\" | wc' # count it\n", + STACK, + pr_is_merged=lambda _n: False, + ) + assert step == ExecStep(command="bash -c 'echo \"$X\" | wc'") + assert step.plan_line(STACK) == ("x bash -c 'echo \"$X\" | wc'", "") + + +@pytest.mark.parametrize( + ("line", "message"), + [ + ("x", "Line 2: unrecognized step: 'x'"), + ("x # nothing to run", "Line 2: unrecognized step"), + ("x echo 'unclosed", "Line 2: malformed command .*No closing quotation"), + # ' #' cuts the line even inside quotes, as for every step. + ("x echo 'a #b'", 'Line 2: malformed command "echo \'a"'), + ], +) +def test_exec_step_parse_errors(line: str, message: str) -> None: + with pytest.raises(ValueError, match=message): + parse_plan(f"l\n{line}\n", STACK, pr_is_merged=lambda _n: False) + + +def test_exec_step_in_the_landed_prefix_is_assumed_completed() -> None: + plan = parse_plan( + "l 98\nx ./provision.sh\nl 99\nx ./smoke-test.sh\nl\n", + STACK, + pr_is_merged=lambda n: n in (98, 99), + ) + assert [s.state for s in plan if isinstance(s, ExecStep)] == [ + "skipped", + "pending", + ] + + +def test_exec_step_replan_credit_needs_the_same_command_after_the_same_prs() -> None: + old_stack = [StackEntry(pr_url="", pr_number=n, branch=f"b{n}") for n in (101, 102)] + old_plan = parse_plan( + "l\nx ./provision.sh\nx ./migrate.sh\nl\n", + old_stack, + pr_is_merged=lambda _n: False, + ) + for step in old_plan[1:3]: + assert isinstance(step, ExecStep) + step.state = "succeeded" + old_plan[2].state = "failed" # type: ignore[attr-defined] + old_stack[0].state = PRState.MERGED + old = LandingContext(stack=old_stack, plan=old_plan, current_step=2) + + stack = old_stack[1:] + plan = parse_plan( + "l 101\nx ./migrate.sh\nx ./provision.sh\nx ./provision.sh --again\nl\n", + stack, + pr_is_merged=lambda n: n == 101, + ) + lost = carry_over_progress(old, plan, stack) + + # Only a finished command carries over, even when reordered; a failed one, + # or a changed command line, runs again. + assert [s.state for s in plan if isinstance(s, ExecStep)] == [ + "pending", + "succeeded", + "pending", + ] + assert lost == [] + + +@pytest.mark.parametrize( + ("state", "outcome", "status", "tone"), + [ + ("pending", StepOutcome.PENDING, "Pending", StepTone.MUTED), + ("running", StepOutcome.ACTIVE, "Running", StepTone.RUNNING), + ("succeeded", StepOutcome.DONE, "Succeeded", StepTone.SUCCESS), + ("failed", StepOutcome.FAILED, "Failed", StepTone.FAILURE), + ("skipped", StepOutcome.DONE, "Assumed completed", StepTone.SUCCESS), + ], +) +def test_exec_view_follows_its_state( + state: str, outcome: StepOutcome, status: str, tone: StepTone +) -> None: + assert _view(ExecStep(command="./provision.sh", state=state)) == StepView( + outcome=outcome, + status=status, + tone=tone, + key="./provision.sh", + description="Run ./provision.sh", + data={"command": "./provision.sh"}, + ) + + +def test_exec_view_detail_is_its_error() -> None: + step = ExecStep( + command="./provision.sh", state="failed", error_message="Command exited 1" + ) + assert _view(step).detail == ("Command exited 1",) diff --git a/tests/test_vscode_extension.py b/tests/test_vscode_extension.py index faa5e6f..915b359 100644 --- a/tests/test_vscode_extension.py +++ b/tests/test_vscode_extension.py @@ -3,7 +3,13 @@ from pathlib import Path from stack_pr.autoland import gh -from stack_pr.autoland.model import ConfirmStep, LandStep, StackEntry, WorkflowStep +from stack_pr.autoland.model import ( + ConfirmStep, + ExecStep, + LandStep, + StackEntry, + WorkflowStep, +) from stack_pr.autoland.plan import ( PLAN_COMMENT_COLUMN, PLAN_COMMENT_GAP, @@ -34,6 +40,7 @@ def test_example_plan_is_accepted_by_the_parser(mocker) -> None: # noqa: ANN001 "LandStep", "WorkflowStep", "ConfirmStep", + "ExecStep", "LandStep", "LandStep", "ConfirmStep", @@ -86,6 +93,7 @@ def test_generated_plan_matches_the_formatter_fixture() -> None: LandStep(entry_index=0, pr_number=101), WorkflowStep(workflow="deploy.yaml"), ConfirmStep(condition="QA sign-off complete"), + ExecStep(command="./scripts/provision.sh --env staging"), LandStep(entry_index=1, pr_number=1024), LandStep(entry_index=2, pr_number=103), ]