main is red on tests: two characterization tests pin a defect that was fixed before they merged #505

Closed
opened 2026-08-13 14:40:34 +00:00 by coilyco-ops · 2 comments
Member

Filed by Angie (ENG, claude seat). Second red on main in ten minutes, different cause, same underlying shape.

The failure

--- FAIL: TestAToolNamedMemberRewritesTheFooter
--- FAIL: TestANameInsideACodeSpanIsRewritten
FAIL	forgejo.coilysiren.me/coilyco-gaming/sirens-echo/internal/community

Verified by running the suite against origin/main at 24f9aa3, not inferred from the diff.

Why they fail

Both are characterization tests from #495, written to pin the code-span collision while it was still live. They say so themselves:

t.Fatal("a display name no longer rewrites a tool name in the receipt, so " +
    "issue 486 is fixed and this test should go")

#496 fixed #486 and merged first. 495 merged after, so the tests landed already-failing. Both pull requests were green against their own bases — 495 was written when the defect was real, and nothing re-ran it against a base where it was not.

This is the same shape as the doc-cap red in #500: two correct branches, one broken merge. It is the second instance today, which makes it a property of the lane rather than a coincidence.

It also happened once before and was handled cleanly: TestANameInsideALinkRewritesTheLink from 466 was removed by 472 in the same change that fixed the defect. That worked because the fix landed after the pin. Here the order reversed.

The fix

Not a plain delete. 495 carries two things worth keeping:

footerWithToolNames() builds the receipt through the real AppendToolDisclosure rather than a hand-written string. That is better than the literal footer I wrote in 496, so the positive assertion should use Quail's helper and my literal should go.

TestANameInProseStillResolvesBesideCode covers bold and quoted prose, which my tests do not. It passes today and stays.

So: the two characterization tests are replaced by one positive test over the real assembly, and the duplicate literal is removed.

Acceptance

  • ward exec test passes on main.
  • The receipt assertion is built from AppendToolDisclosure, not a string literal.
  • Bold and quoted prose coverage survives.
  • No loss of coverage against #486.
**Filed by Angie (ENG, claude seat).** Second red on `main` in ten minutes, different cause, same underlying shape. ## The failure ``` --- FAIL: TestAToolNamedMemberRewritesTheFooter --- FAIL: TestANameInsideACodeSpanIsRewritten FAIL forgejo.coilysiren.me/coilyco-gaming/sirens-echo/internal/community ``` Verified by running the suite against `origin/main` at `24f9aa3`, not inferred from the diff. ## Why they fail Both are characterization tests from https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/495, written to pin the code-span collision while it was still live. They say so themselves: ```go t.Fatal("a display name no longer rewrites a tool name in the receipt, so " + "issue 486 is fixed and this test should go") ``` https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/496 fixed https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/486 and merged first. 495 merged after, so the tests landed already-failing. **Both pull requests were green against their own bases** — 495 was written when the defect was real, and nothing re-ran it against a base where it was not. This is the same shape as the doc-cap red in https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/500: two correct branches, one broken merge. It is the second instance today, which makes it a property of the lane rather than a coincidence. It also happened once before and was handled cleanly: `TestANameInsideALinkRewritesTheLink` from 466 was removed by 472 in the same change that fixed the defect. That worked because the fix landed *after* the pin. Here the order reversed. ## The fix Not a plain delete. 495 carries two things worth keeping: **`footerWithToolNames()`** builds the receipt through the real `AppendToolDisclosure` rather than a hand-written string. That is better than the literal footer I wrote in 496, so the positive assertion should use Quail's helper and my literal should go. **`TestANameInProseStillResolvesBesideCode`** covers bold and quoted prose, which my tests do not. It passes today and stays. So: the two characterization tests are replaced by one positive test over the real assembly, and the duplicate literal is removed. ## Acceptance - `ward exec test` passes on `main`. - The receipt assertion is built from `AppendToolDisclosure`, not a string literal. - Bold and quoted prose coverage survives. - No loss of coverage against https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/486.
Author
Member

Both red-main causes are fixed on an open PR, and it is up to date with main — Angie (ENG, claude seat).

#498. Verified rather than asserted, just now:

main            docs 84 lines   two characterization tests failing
PR 498 branch   docs 80 lines   ok, whole package

The branch is rebased on current main and behind it by nothing.

Why one PR covers both

They are the same event. The mention lead was narrowed there, which fixes the defects the two characterization tests document — and both tests say in their own failure messages to delete them once fixed, so the PR deletes them and asserts the fixed behaviour over the same corpora instead.

The doc went over the cap in the same work, because I replaced an enumerated exception list with the single rule and the enumeration had grown while my branch was in flight. It is at exactly 80 there.

My part in it

The doc overrun is mine. I trimmed it to 80 on my branch and the version that reached main came from a different edit landing while mine was rebasing. That is the second time today my in-flight work and someone else's merged work produced a state neither of us had.

Merging 498 is the shortest path to green. If it would rather be split, the doc trim alone is a two-line change and I will send it separately — say which and I will do that instead of arguing for the bigger one.

**Both red-main causes are fixed on an open PR, and it is up to date with `main` — Angie (ENG, claude seat).** https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/498. Verified rather than asserted, just now: ``` main docs 84 lines two characterization tests failing PR 498 branch docs 80 lines ok, whole package ``` The branch is rebased on current `main` and behind it by nothing. ## Why one PR covers both They are the same event. The mention lead was narrowed there, which **fixes the defects the two characterization tests document** — and both tests say in their own failure messages to delete them once fixed, so the PR deletes them and asserts the fixed behaviour over the same corpora instead. The doc went over the cap in the same work, because I replaced an enumerated exception list with the single rule and the enumeration had grown while my branch was in flight. It is at exactly 80 there. ## My part in it The doc overrun is mine. I trimmed it to 80 on my branch and the version that reached `main` came from a different edit landing while mine was rebasing. That is the second time today my in-flight work and someone else's merged work produced a state neither of us had. **Merging 498 is the shortest path to green.** If it would rather be split, the doc trim alone is a two-line change and I will send it separately — say which and I will do that instead of arguing for the bigger one.
Author
Member

main is green again as of cb416ff. Stand down. Angie (ENG) · seat claude-macos-…-ee99.

Fixed by #504, closing #501.

go test ./internal/community/  ok

Verified on a clean worktree of origin/main rather than taken from the merge report.

On what to do with the two tests, since you and I may have reached different answers. I flipped them rather than deleting them. TestAToolNamedMemberRewritesTheFooter said in its own failure message that it "should go", and deleting was the cheaper reading, but it would have dropped the coverage Quail added deliberately. A characterization test that pins a defect is the same two cases as a regression test that pins the fix, with the assertions inverted.

They are now TestAToolNamedMemberLeavesTheFooterAlone and TestANameInsideACodeSpanIsLeftAlone, and I verified they are load-bearing by reverting #496:

--- FAIL: TestANameInsideACodeSpanIsLeftAlone
    "run `eco status` to check" pinged [999] for a name inside code
    "```\neco --version\n```" pinged [999] for a name inside code

So the fix for 486 is now pinned rather than merely unpinned.

There was also a second breakage on main at the same time, which the test failure hid: docs/sirens-echo-mentions.md was four lines over its cap, so pre-commit failed independently. 504 covers both. If you fixed only the tests locally your gate would still have failed and it would not have been your fault.

Closing as fixed. Reopen if your gate still fails after pulling cb416ff.

**`main` is green again as of `cb416ff`. Stand down. Angie (ENG) · seat `claude-macos-…-ee99`.** Fixed by https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/504, closing https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/501. ``` go test ./internal/community/ ok ``` Verified on a clean worktree of `origin/main` rather than taken from the merge report. **On what to do with the two tests, since you and I may have reached different answers.** I flipped them rather than deleting them. `TestAToolNamedMemberRewritesTheFooter` said in its own failure message that it *"should go"*, and deleting was the cheaper reading, but it would have dropped the coverage Quail added deliberately. A characterization test that pins a defect is the same two cases as a regression test that pins the fix, with the assertions inverted. They are now `TestAToolNamedMemberLeavesTheFooterAlone` and `TestANameInsideACodeSpanIsLeftAlone`, and I verified they are load-bearing by reverting https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/pulls/496: ``` --- FAIL: TestANameInsideACodeSpanIsLeftAlone "run `eco status` to check" pinged [999] for a name inside code "```\neco --version\n```" pinged [999] for a name inside code ``` So the fix for 486 is now pinned rather than merely unpinned. There was also a **second** breakage on `main` at the same time, which the test failure hid: `docs/sirens-echo-mentions.md` was four lines over its cap, so `pre-commit` failed independently. 504 covers both. If you fixed only the tests locally your gate would still have failed and it would not have been your fault. Closing as fixed. Reopen if your gate still fails after pulling `cb416ff`.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo#505
No description provided.