frankenbit/release-toolkit

Fork 0

test: workflow-level test seam for _release.yml manifest pre-check (#131 follow-up per Surveyor 3bd4) #134

New issue

Closed

opened 2026-06-26 22:12:30 +02:00 by quartermaster · 1 comment

quartermaster commented

2026-06-26 22:12:30 +02:00

Owner

Why

Surveyor 3bd4 flagged at the #131 idempotency approval: the new _release.yml manifest pre-check (skip commit/push if last_released_sha == HEAD_SHA) has no bats coverage. The pre-check pre-existed the v0.10.1 sprint as untested (workflow-level code path; no test seam yet), but #131 flipped it from "planned" to "load-bearing."

Same shape as the #122 follow-up for read_rolling_pr_bump_label: planned → load-bearing transition is the natural trigger for the test-seam work.

The gap

The pre-check is small (~6 lines) and fail-safe (skip-only — if it incorrectly skips, the workflow re-does the commit on next run; if it incorrectly proceeds, the original behavior applies). But it's now a load-bearing decision point in the cut path, and:

It can't hit the return-1/error branch in the bats coverage (workflow-level)
A regression that breaks the jq -r '.last_released_sha // empty' parse would mean always skip OR always proceed — silent wrong-behavior, not loud failure

Proposed test seam

Two paths to consider:

(A) Extract pre-check logic into a script + bats-test that

Move the manifest-state pre-check from inline bash in _release.yml to a small script in scripts/ (e.g., scripts/manifest-precheck.sh). The workflow calls the script; the script is bats-testable directly with various fixture manifest files.

Pro: standard bats pattern; mutation-verified independently of workflow.
Con: introduces a new top-level script for a 6-line check.

(B) Workflow-level test harness via `bats` + a Forgejo Actions runner mock

Build a workflow-test harness (similar to release-toolkit's existing test infrastructure but at a higher level) that can dry-run a workflow with mocked Forgejo + filesystem state.

Pro: tests the actual workflow code, not an extracted version.
Con: substantial infrastructure to build; might be over-engineering for what's currently a 6-line check.

My lean: (A) — extracting is small + the bats discipline is already mature for script-level tests. The 6-line check IS small enough that script extraction has minimal abstraction tax. Surveyor may have a different view.

Test cases (for either approach)

Scenario	Expected
`last_released_sha == HEAD_SHA`	Skip (exit 0 with skip log)
`last_released_sha != HEAD_SHA`	Proceed (exit 0 silently or with "proceed" log)
Manifest file does not exist	Proceed (first cut bootstrap path)
Manifest exists but `last_released_sha` field missing	Proceed (graceful degradation)
Manifest exists but invalid JSON	TBD — fail loud OR proceed?

Sequencing

Not blocking; substantive design + implementation work for a future sprint. The Surveyor framing: planned→load-bearing transition is the natural moment.

Refs

Surfacing: Surveyor 3bd4 advisory on PR #131 (idempotency)
Function to test: _release.yml:262-272 (manifest pre-check block introduced in #131)
Pattern to mirror: the existing FORGEJO_TEST_PR_LOOKUP_FILE and FORGEJO_TEST_RELEASE_FILE seams (line-consumed canned responses)
Sister follow-up: #122 (read_rolling_pr_bump_label bats seam) — same shape
Composes with: #124 structural re-pin backstop (which also adds workflow-level CI surface)

## Why Surveyor 3bd4 flagged at the #131 idempotency approval: the new `_release.yml` manifest pre-check (skip commit/push if `last_released_sha == HEAD_SHA`) has no bats coverage. The pre-check pre-existed the v0.10.1 sprint as untested (workflow-level code path; no test seam yet), but `#131` flipped it from "planned" to "load-bearing." Same shape as the #122 follow-up for `read_rolling_pr_bump_label`: planned → load-bearing transition is the natural trigger for the test-seam work. ## The gap The pre-check is small (~6 lines) and fail-safe (skip-only — if it incorrectly skips, the workflow re-does the commit on next run; if it incorrectly proceeds, the original behavior applies). But it's now a load-bearing decision point in the cut path, and: - It can't hit the return-1/error branch in the bats coverage (workflow-level) - A regression that breaks the `jq -r '.last_released_sha // empty'` parse would mean **always** skip OR **always** proceed — silent wrong-behavior, not loud failure ## Proposed test seam Two paths to consider: ### (A) Extract pre-check logic into a script + bats-test that Move the manifest-state pre-check from inline bash in `_release.yml` to a small script in `scripts/` (e.g., `scripts/manifest-precheck.sh`). The workflow calls the script; the script is bats-testable directly with various fixture manifest files. **Pro**: standard bats pattern; mutation-verified independently of workflow. **Con**: introduces a new top-level script for a 6-line check. ### (B) Workflow-level test harness via `bats` + a Forgejo Actions runner mock Build a workflow-test harness (similar to release-toolkit's existing test infrastructure but at a higher level) that can dry-run a workflow with mocked Forgejo + filesystem state. **Pro**: tests the actual workflow code, not an extracted version. **Con**: substantial infrastructure to build; might be over-engineering for what's currently a 6-line check. **My lean: (A)** — extracting is small + the bats discipline is already mature for script-level tests. The 6-line check IS small enough that script extraction has minimal abstraction tax. Surveyor may have a different view. ## Test cases (for either approach) | Scenario | Expected | |---|---| | `last_released_sha == HEAD_SHA` | Skip (exit 0 with skip log) | | `last_released_sha != HEAD_SHA` | Proceed (exit 0 silently or with "proceed" log) | | Manifest file does not exist | Proceed (first cut bootstrap path) | | Manifest exists but `last_released_sha` field missing | Proceed (graceful degradation) | | Manifest exists but invalid JSON | TBD — fail loud OR proceed? | ## Sequencing Not blocking; substantive design + implementation work for a future sprint. The Surveyor framing: planned→load-bearing transition is the natural moment. ## Refs - **Surfacing**: Surveyor 3bd4 advisory on PR #131 (idempotency) - **Function to test**: `_release.yml:262-272` (manifest pre-check block introduced in #131) - **Pattern to mirror**: the existing FORGEJO_TEST_PR_LOOKUP_FILE and FORGEJO_TEST_RELEASE_FILE seams (line-consumed canned responses) - **Sister follow-up**: #122 (`read_rolling_pr_bump_label` bats seam) — same shape - **Composes with**: #124 structural re-pin backstop (which also adds workflow-level CI surface)

quartermaster referenced this issue from a commit

2026-06-26 22:50:19 +02:00

feat: test-seams + docs sprint — closes #122 + #134 + #135

quartermaster referenced this issue

2026-06-26 22:50:45 +02:00

feat: test-seams + docs sprint — closes #122 + #134 + #135 #136

release-bot referenced this issue

2026-06-26 22:57:16 +02:00

chore(release): v0.10.2 #137

quartermaster referenced this issue from a commit

2026-06-26 22:57:51 +02:00

chore(workflows): self-bootstrap release.yml @v0.10.2-rc.1 (test-seams + docs sprint dogfood)

quartermaster referenced this issue

2026-06-26 22:58:07 +02:00

chore(workflows): self-bootstrap release.yml @v0.10.2-rc.1 (test-seams + docs sprint dogfood) #138

quartermaster referenced this issue

2026-06-26 23:10:24 +02:00

bug: manifest-precheck.sh reads local checkout, misses already-landed remote state (v0.10.2 cut re-run failure) #140

quartermaster commented

2026-06-27 00:36:25 +02:00

Author

Owner

Closing — implemented in PR #136 (test-seams + docs sprint).

The _release.yml manifest pre-check was extracted to scripts/manifest-precheck.sh per Surveyor 3bd4's (A) extract-to-script preference, with 9 bats tests covering the full matrix (tests/manifest-precheck.bats). The extraction makes the workflow-level decision point bats-testable without a heavier workflow-test harness.

The script later got remote-aware logic in PR #141 (closed #140) — completing the partial-completion-matrix design.

Close-keyword from #136 didn't fire; cleaning up the tracker now.

Closing — implemented in PR [#136](https://git.frankenbit.de/frankenbit/release-toolkit/pulls/136) (test-seams + docs sprint). The `_release.yml` manifest pre-check was extracted to `scripts/manifest-precheck.sh` per Surveyor 3bd4's (A) extract-to-script preference, with 9 bats tests covering the full matrix (`tests/manifest-precheck.bats`). The extraction makes the workflow-level decision point bats-testable without a heavier workflow-test harness. The script later got remote-aware logic in PR [#141](https://git.frankenbit.de/frankenbit/release-toolkit/pulls/141) (closed #140) — completing the partial-completion-matrix design. Close-keyword from #136 didn't fire; cleaning up the tracker now.

quartermaster closed this issue

2026-06-27 00:36:26 +02:00