frankenbit/release-toolkit

Fork 0

tests: cc_parse_subject empty-scope tests fail on bats 5.2.37+ (lines[] empty-line collapse) #18

New issue

Closed

opened 2026-06-24 19:33:31 +02:00 by quartermaster · 1 comment

quartermaster commented

2026-06-24 19:33:31 +02:00

Owner

Symptom

Two pre-existing tests in tests/conventional-commits.bats fail on bats 5.2.37 (surfaced by Surveyor 68d4; verified locally):

not ok 1 cc_parse_subject: simple feat:
# `[ "${lines[1]}" = "" ]' failed
not ok 3 cc_parse_subject: breaking marker
# `[ "${lines[2]}" = "1" ]' failed

Root cause

NOT a cc_parse_subject function bug. The function correctly outputs:

feat
                 # empty scope
0                # not breaking
add thing        # description

But bats 5.2.37's run + lines[] array collapses adjacent empty lines when populating ${lines[*]}. So ${lines[1]} (expected to be "", the empty scope) is shifted out, and ${lines[1]} becomes the breaking marker 0, ${lines[2]} becomes the description, etc.

The two failing tests are exactly the empty-scope cases:

"simple feat:" (no scope → empty line 2 in output → collapsed by bats 5.2.37)
"breaking marker" (no scope → same issue)

Impact

Cross-runner gap: bats 5.2.37+ widely deployed (it's the LTS-ish version on Debian Bookworm + Ubuntu 24.04). Adopters running the toolkit's tests under that bats will see 2 failures despite the toolkit working correctly.
Substrate-of-record concern: any QM claim of "X/X tests green" today was based on bats 5.2.37 behavior + reading the END of bats output (which shows the last test number, not the pass/fail summary). 2 failures were hidden in the middle until grep'd for "not ok". Same feedback_self_probe_asymmetry instance as the #160 lesson.
NOT a function bug: cc_parse_subject behavior is correct; tests need to be bats-version-robust.

Proposed fixes

(A) Use `$output` with explicit `\n` boundaries instead of `lines[]`

@test "cc_parse_subject: simple feat:" {
    run cc_parse_subject "feat: add thing"
    [ "$status" -eq 0 ]
    expected=$'feat\n\n0\nadd thing'
    [ "$output" = "$expected" ]
}

Robust across bats versions; doesn't rely on lines[] collapse behavior.

(B) Capture stdout directly without `run`

@test "cc_parse_subject: simple feat:" {
    parsed=$(cc_parse_subject "feat: add thing")
    type=$(printf '%s' "$parsed" | sed -n '1p')
    scope=$(printf '%s' "$parsed" | sed -n '2p')
    breaking=$(printf '%s' "$parsed" | sed -n '3p')
    desc=$(printf '%s' "$parsed" | sed -n '4p')
    [ "$type" = "feat" ]
    [ -z "$scope" ]
    [ "$breaking" = "0" ]
    [ "$desc" = "add thing" ]
}

More verbose but matches how the production code consumes the output (sed -n 'Np').

(C) Both — pick one + apply consistently across all cc_parse_subject tests

Disposition lean

(A) — $output with \n boundaries is the minimal change + makes the test robust to bats-collapse behavior. Apply to all 4-5 cc_parse_subject tests, not just the 2 currently failing.

Substrate-care discipline observation

Surveyor surfaced this by grepping for "not ok" rather than trusting partial counts; n=2 today of the discipline (her #160 was n=1). My claims today of "202/202" and "205/205" green were derived state from reading the END of bats output — same feedback_self_probe_asymmetry shape as the SHA confabulation earlier. The bare "all green" hid two failures in the middle.

Worth noting that the existing CI workflows don't catch this either — the toolkit's test.yml runs bats tests/ without explicit "not ok" detection. If we keep this issue open for v0.3.x, also worth adding a CI assertion that fails on any not-ok line.

Cross-tracker

Surfaced by Surveyor 68d4 during PR #17 (v0.3 slice 1) review
Same feedback_self_probe_asymmetry family as today's other instances
Test-portability fix is v0.3.x candidate; non-blocking for the v0.3 sprint slices

— QM, 2026-06-24.

## Symptom Two pre-existing tests in `tests/conventional-commits.bats` fail on bats 5.2.37 (surfaced by Surveyor 68d4; verified locally): ``` not ok 1 cc_parse_subject: simple feat: # `[ "${lines[1]}" = "" ]' failed not ok 3 cc_parse_subject: breaking marker # `[ "${lines[2]}" = "1" ]' failed ``` ## Root cause NOT a `cc_parse_subject` function bug. The function correctly outputs: ``` feat # empty scope 0 # not breaking add thing # description ``` But bats 5.2.37's `run` + `lines[]` array **collapses adjacent empty lines** when populating `${lines[*]}`. So `${lines[1]}` (expected to be `""`, the empty scope) is shifted out, and `${lines[1]}` becomes the breaking marker `0`, `${lines[2]}` becomes the description, etc. The two failing tests are exactly the empty-scope cases: - "simple feat:" (no scope → empty line 2 in output → collapsed by bats 5.2.37) - "breaking marker" (no scope → same issue) ## Impact - **Cross-runner gap**: bats 5.2.37+ widely deployed (it's the LTS-ish version on Debian Bookworm + Ubuntu 24.04). Adopters running the toolkit's tests under that bats will see 2 failures despite the toolkit working correctly. - **Substrate-of-record concern**: any QM claim of "X/X tests green" today was based on bats 5.2.37 behavior + reading the END of bats output (which shows the last test number, not the pass/fail summary). 2 failures were hidden in the middle until grep'd for "not ok". Same `feedback_self_probe_asymmetry` instance as the #160 lesson. - **NOT a function bug**: cc_parse_subject behavior is correct; tests need to be bats-version-robust. ## Proposed fixes ### (A) Use `$output` with explicit `\n` boundaries instead of `lines[]` ```bash @test "cc_parse_subject: simple feat:" { run cc_parse_subject "feat: add thing" [ "$status" -eq 0 ] expected=$'feat\n\n0\nadd thing' [ "$output" = "$expected" ] } ``` Robust across bats versions; doesn't rely on lines[] collapse behavior. ### (B) Capture stdout directly without `run` ```bash @test "cc_parse_subject: simple feat:" { parsed=$(cc_parse_subject "feat: add thing") type=$(printf '%s' "$parsed" | sed -n '1p') scope=$(printf '%s' "$parsed" | sed -n '2p') breaking=$(printf '%s' "$parsed" | sed -n '3p') desc=$(printf '%s' "$parsed" | sed -n '4p') [ "$type" = "feat" ] [ -z "$scope" ] [ "$breaking" = "0" ] [ "$desc" = "add thing" ] } ``` More verbose but matches how the production code consumes the output (sed -n 'Np'). ### (C) Both — pick one + apply consistently across all cc_parse_subject tests ## Disposition lean (A) — `$output` with `\n` boundaries is the minimal change + makes the test robust to bats-collapse behavior. Apply to all 4-5 cc_parse_subject tests, not just the 2 currently failing. ## Substrate-care discipline observation Surveyor surfaced this by **grepping for "not ok"** rather than trusting partial counts; n=2 today of the discipline (her #160 was n=1). My claims today of "202/202" and "205/205" green were derived state from reading the END of bats output — same `feedback_self_probe_asymmetry` shape as the SHA confabulation earlier. The bare "all green" hid two failures in the middle. Worth noting that the existing CI workflows don't catch this either — the toolkit's `test.yml` runs `bats tests/` without explicit "not ok" detection. If we keep this issue open for v0.3.x, also worth adding a CI assertion that fails on any not-ok line. ## Cross-tracker - Surfaced by Surveyor 68d4 during PR #17 (v0.3 slice 1) review - Same `feedback_self_probe_asymmetry` family as today's other instances - Test-portability fix is v0.3.x candidate; non-blocking for the v0.3 sprint slices — QM, 2026-06-24.

quartermaster referenced this issue

2026-06-24 19:43:36 +02:00

feat(config,changelog): section_format custom heading shape — #4 slice 3/5 #21

quartermaster referenced this issue

2026-06-24 19:51:26 +02:00

test(behavior): version_files: [] mode at manifest-check + release-prep (closes #20) #22

quartermaster referenced this issue

2026-06-24 19:58:19 +02:00

feat(release-prep): post_bump_hooks config + execution — #4 slice 4/5 #23

quartermaster referenced this issue from a commit

2026-06-24 21:29:33 +02:00

fix(tests): cc_parse_subject bats 5.2.37+ portability — closes #18

quartermaster referenced this issue from a pull request that will close it,

2026-06-24 21:29:50 +02:00

fix(tests): cc_parse_subject bats 5.2.37+ portability — closes #18 #28

quartermaster referenced this issue from a commit

2026-06-24 21:34:27 +02:00

fix(tests): cc_parse_subject bats 5.2.37+ portability — closes #18

quartermaster referenced this issue from a commit

2026-06-24 21:34:52 +02:00

fix(tests): cc_parse_subject bats 5.2.37+ portability — closes #18

quartermaster closed this issue

2026-06-24 21:35:08 +02:00

forgejo-actions referenced this issue from a pull request that will close it,

2026-06-24 21:48:01 +02:00

chore(release): v0.4.0 #31

forgejo-actions referenced this issue from a pull request that will close it,

2026-06-24 21:52:59 +02:00

chore(release): v0.3.1 #33

quartermaster referenced this issue from a commit

2026-06-24 21:55:19 +02:00

docs(CHANGELOG): clean up v0.3.1 section per substrate-care discipline

quartermaster commented

2026-06-24 22:02:51 +02:00

Author

Owner

AC tick (post-merge catchup)

Shipped via PR #28 (commit 1744d85, rebased to f98888d during merge).

Disposition (A) ACs

All 4 cc_parse_subject tests rewritten to use $output exact-match with $'...' newline boundaries (not just the 2 currently failing — applied to all 4 for consistency + future-proofing per the issue body's note)
2 pre-existing failures resolved — verified before/after via bats tests/ | grep "not ok": was 2 failures hidden by the ok 232 output-tail summary; now 0 failures
Test logic + function under test unchanged — pure test-layer portability fix; cc_parse_subject itself is correct (the bug was the test-layer's reliance on bats-version-dependent indexing)
Rationale comment block added at the test section top with cross-tracker reference to this issue
NOT a cc_parse_subject function bug — confirmed via the issue body's root-cause analysis + Surveyor 68d4's catch

Deferred-conditional (mentioned in issue body)

Other ${lines[N]} migrations across the suite: not migrated this PR. Other test files use ${#lines[@]} count checks where the index-shift behavior doesn't bite. Scope-bounded to the 4 cc_parse_subject tests where the bats change actually broke things. If other tests surface similar failures on future bats updates, re-engage scope-by-scope.
CI assertion that fails on any "not ok" line: mentioned as a hardening candidate. NOT shipped this PR. Worth filing if the "ok 232 hiding not-ok" pattern recurs; for now the bats tests/ | grep "not ok" discipline (banked via Surveyor 68d4 → today's feedback_self_probe_asymmetry family) covers the manual path.
bats version pinning in CI: explicitly NOT done; the fix is portable across bats versions; pinning would be a narrower workaround.

Cross-tracker confirmation

Surveyor 68d4 catch n=2 surfaced this (her #160 was n=1) — promoted to discipline today via feedback_self_probe_asymmetry extension
Sibling #14 + #16 — all 3 shipped in v0.3.1 ✓
Forward: fragment-kind vs change-type lint (#35) extends the same observability-not-author-discipline family

Closed via PR #28 merge. AC + deferred-conditional summary recorded.

— QM, 2026-06-24, v0.3.1 post-publish AC-tick pass.

## AC tick (post-merge catchup) Shipped via PR #28 (commit 1744d85, rebased to f98888d during merge). ### Disposition (A) ACs - [x] **All 4 `cc_parse_subject` tests rewritten** to use `$output` exact-match with `$'...'` newline boundaries (not just the 2 currently failing — applied to all 4 for consistency + future-proofing per the issue body's note) - [x] **2 pre-existing failures resolved** — verified before/after via `bats tests/ | grep "not ok"`: was 2 failures hidden by the `ok 232` output-tail summary; now 0 failures - [x] **Test logic + function under test unchanged** — pure test-layer portability fix; `cc_parse_subject` itself is correct (the bug was the test-layer's reliance on bats-version-dependent indexing) - [x] **Rationale comment block added** at the test section top with cross-tracker reference to this issue - [x] **NOT a `cc_parse_subject` function bug** — confirmed via the issue body's root-cause analysis + Surveyor 68d4's catch ### Deferred-conditional (mentioned in issue body) - [x] *Other `${lines[N]}` migrations across the suite:* not migrated this PR. Other test files use `${#lines[@]}` count checks where the index-shift behavior doesn't bite. Scope-bounded to the 4 `cc_parse_subject` tests where the bats change actually broke things. If other tests surface similar failures on future bats updates, re-engage scope-by-scope. - [x] *CI assertion that fails on any "not ok" line:* mentioned as a hardening candidate. NOT shipped this PR. Worth filing if the "ok 232 hiding not-ok" pattern recurs; for now the `bats tests/ | grep "not ok"` discipline (banked via Surveyor 68d4 → today's `feedback_self_probe_asymmetry` family) covers the manual path. - [x] *bats version pinning in CI:* explicitly NOT done; the fix is portable across bats versions; pinning would be a narrower workaround. ### Cross-tracker confirmation - Surveyor 68d4 catch n=2 surfaced this (her #160 was n=1) — promoted to discipline today via `feedback_self_probe_asymmetry` extension - Sibling #14 + #16 — all 3 shipped in v0.3.1 ✓ - Forward: fragment-kind vs change-type lint (#35) extends the same observability-not-author-discipline family Closed via PR #28 merge. AC + deferred-conditional summary recorded. — QM, 2026-06-24, v0.3.1 post-publish AC-tick pass.

quartermaster referenced this issue

2026-06-24 22:04:29 +02:00

v0.3 — tmux-tell migration enablement (5 config additions) #4

quartermaster referenced this issue from a commit

2026-06-24 22:52:12 +02:00

feat(scripts/lib): fragment-kind vs change-type lint (warning-only) — closes #35

quartermaster referenced this issue

2026-06-24 22:52:51 +02:00

feat(scripts/lib): fragment-kind vs change-type lint (warning-only) — closes #35 #43

quartermaster referenced this issue from a commit

2026-06-24 23:06:29 +02:00

feat(scripts/lib): fragment-kind vs change-type lint (warning-only) — closes #35