fix(test): decouple a second logger assertion from log formatting (#2681)
Second instance of the defect fixed in #2675 — found by applying the same attribution pass to the **core** suite's long-red files rather than counting them. ## The defect `withSeverityMarker` (`logger.ts:31`) deliberately wraps every message in a machine-readable severity marker so the TUI log pane can colour by level: an `fnlvl=<level>` marker plus a `[core-merge-policy]` subsystem tag ahead of the real text. This case pinned the raw string with `toHaveBeenCalledWith`, so it broke when that convention landed. It was coupled to log **formatting**, not to the behaviour it exists to check. ## Two instances is a pattern `toHaveBeenCalledWith` on a logger is brittle **by construction** in this codebase, because decorating the message is the logger's entire job. Any assertion pinning an exact logged string will break the next time the format changes — and both instances found so far were long-red, i.e. nobody noticed they had stopped testing anything. Worth a lint rule or a shared helper if a third appears. I have not added one for two instances. ## What is preserved Warn-once semantics and the requirement that the warning names both the legacy value and its replacement are unchanged and still fully asserted. Only the exact-prefix coupling is removed. **Mutation-verified:** deleting the `severityAuditLog.warn` call in `merge-policy.ts` fails with `expected "warn" to be called 1 times, but got 0 times`. A contains-check that passed because it matched nothing would be worse than the brittle assertion it replaces. ## Still red in the core suite, not addressed here Two neighbours in the same cluster, both needing an owner's context rather than a guess: - `settings-parity.test.ts` — `expected [ 'testMode', 'voiceInput', …(15) ] to deeply equal [ …(14) ]`. A settings key now appears in **both** global and project scope without being listed as intentional. That is either a real scoping mistake or a stale allowlist, and the difference matters. - `workflow-ir-settings.test.ts` — `expected 10 to strictly equal 3` on the moved-key catalog. Both are genuine signals, not noise. Flagging rather than guessing, same as the funnel decision on #2674. ## Verification `settings-defaults.test.ts` **39/40 → 40/40**. `pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm check:lifecycle-columns` exits 0. `pnpm lint` clean. Test-only; no changeset. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -304,8 +304,23 @@ describe("settings defaults invariants", () => {
|
||||
expect(normalizeMergeIntegrationWorktreeMode("cwd-main")).toBe("cwd-integration-branch");
|
||||
expect(normalizeMergeIntegrationWorktreeMode("cwd-main")).toBe("cwd-integration-branch");
|
||||
|
||||
/*
|
||||
FNXC:EngineDiagnostics 2026-07-30-18:00:
|
||||
Asserted with a CONTAINS check, because the logger deliberately wraps every message in a
|
||||
machine-readable severity marker — `withSeverityMarker` (logger.ts:31) prepends an
|
||||
`fnlvl=<level>` marker plus a `[core-merge-policy]` subsystem tag. Pinning the raw string
|
||||
coupled this case to log FORMATTING rather than to the behaviour it exists to check, so it
|
||||
broke when that convention landed.
|
||||
|
||||
Same defect and same fix as the audit-emitter assertion in central-archive-secrets (PR #2675).
|
||||
Two instances is a pattern: `toHaveBeenCalledWith` on a logger is brittle by construction here,
|
||||
because the logger's job is to decorate the message.
|
||||
|
||||
What this case actually cares about — warn-once semantics, and that the warning names the legacy
|
||||
value and its replacement — is unchanged and still fully asserted.
|
||||
*/
|
||||
expect(warnSpy).toHaveBeenCalledTimes(1);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect(String(warnSpy.mock.calls[0]![0])).toContain(
|
||||
"[merger] settings.mergeIntegrationWorktree=cwd-main is legacy; normalized to cwd-integration-branch",
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user