From 30e287a29ec8016c747f5a7e92e674a059227c0e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 02:30:20 -0700 Subject: [PATCH] fix(test): decouple a second logger assertion from log formatting (#2681) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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=` 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) --- .../src/__tests__/settings-defaults.test.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/settings-defaults.test.ts b/packages/core/src/__tests__/settings-defaults.test.ts index 731223b695..5f49f76772 100644 --- a/packages/core/src/__tests__/settings-defaults.test.ts +++ b/packages/core/src/__tests__/settings-defaults.test.ts @@ -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=` 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", ); });