* test(#3314): backfill deterministic clock-seam coverage (failing-first) Replaces loose regex/range assertions with exact-value and boundary tests for the CLI-subprocess and in-process clock-touching call sites identified by H2's audit (epic #3053): cmdCurrentTimestamp, _wsParseRetryAfter (commands.cts), cmdInitManager's is_active gate and cmdInitQuick's quick_id generation (init.cts), and reapStaleTempFiles (io.cts). The CLI-subprocess-pinned tests are expected RED until a follow-up commit routes those call sites through realClock so GSD_TEST_MODE+GSD_NOW_MS can reach them. * refactor(#3314): route CLI-subprocess clock reads through realClock cmdCurrentTimestamp, cmdInitManager's is_active gate, and cmdInitQuick's quick_id generation read Date directly, which the GSD_TEST_MODE+GSD_NOW_MS subprocess pin cannot reach (it only fires inside realClock.now()). Behavior-preserving: realClock.now() falls through to Date.now() whenever GSD_TEST_MODE is unset, which is every real invocation. * docs(#3314): amend ADR-456 with reachability-based clock-control rule ADR-456 §(a) documented one mechanism (injected {clock=Date} + t.mock.timers). Adds the two this repo already relies on: t.mock.timers for in-process direct-Date reads, and the GSD_TEST_MODE+GSD_NOW_MS subprocess pin (routed through realClock) for CLI-spawned code. Updates TESTING-STANDARDS.md's matching passages, which already referenced this issue by number as the no-elapsed-assertion promotion precondition. * fix(#3314): address orthogonal review findings Spec-axis findings: file and link the no-elapsed-assertion promotion follow-up (#3331) instead of leaving TESTING-STANDARDS.md pointing at a dead #1885, and ship the module-by-module audit table in the ADR itself rather than only in a gitignored phase artifact. Standards-axis finding: pin the "hour-old file = not active" test via GSD_TEST_MODE+GSD_NOW_MS for consistency with the reachability rule this PR's own ADR amendment now documents. --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -53,6 +53,27 @@ Real multi-process race tests (where two OS processes genuinely compete for a re
|
||||
|
||||
Wall-clock timing in production code paths that cannot accept an injectable clock (e.g., third-party integrations) must be wrapped behind an adapter interface so tests can substitute a controlled clock.
|
||||
|
||||
#### Reachability-based mechanism selection (amended 2026-08-10, #3314)
|
||||
|
||||
As originally written, this section names one mechanism — an injected `{clock = Date}` parameter driven by `t.mock.timers`. In practice this repo uses three, and the choice between them is determined by how a test reaches the system under test, not by preference:
|
||||
|
||||
| Test reaches the SUT via… | Mechanism | Why |
|
||||
|---|---|---|
|
||||
| **Direct in-process call** (`require(...)` then invoke) — regardless of whether the SUT accepts an injected clock or reads a global | `t.mock.timers.enable(['Date'])` | Patches the process-global `Date` class. Any code in the same process — whether it reads `Date.now()` directly or through `realClock.now()` — observes the mocked time. No production code change is required to make an in-process-tested function deterministic. |
|
||||
| **Spawned CLI subprocess** (`execFileSync`/`spawnSync` into a new Node process) | `GSD_TEST_MODE=1` + `GSD_NOW_MS=<epoch-ms>` (subprocess time-pin adapter, issue #474), with `TZ` also pinned when local-time getters are involved | `t.mock.timers` in the parent test process cannot reach a child process's clock — mocking is process-local. `GSD_NOW_MS` is read inside `realClock.now()`'s `_pinnedNowMs()` check (`src/clock.cts`), so it reaches the subprocess's clock **only if the subprocess's code path reads time through `realClock`** (`now()`/`nowIso()`/`today()`/`localToday()`). A code path that calls raw `Date.now()`/`new Date()` bypasses the pin entirely and stays untestable no matter what the test does — this is the concrete, repo-specific form the preceding paragraph's "must be wrapped behind an adapter interface" requirement takes at the subprocess boundary. `realClock.now() === Date.now()` whenever `GSD_TEST_MODE` is unset, so routing a call site through the seam is behavior-preserving in production. |
|
||||
| Module **accepts** an injected `{clock = Date}` parameter | `makeFakeClock()` (`tests/helpers/clock.cjs`) | The pattern this section already documented — unchanged, still correct for this population. |
|
||||
|
||||
This does not relax the typed-surface or delete-bad-tests policies elsewhere in this ADR; it only names the two mechanisms the original text omitted. Their absence from the documented policy is why several `src` modules read as "zero time-control adoption" despite some already routing through `realClock` correctly for part of their output. #3314's audit of the 10 originally-flagged direct-use modules:
|
||||
|
||||
| Module | Classification | Rationale |
|
||||
|---|---|---|
|
||||
| `commands.cts` | Backfilled | `cmdCurrentTimestamp`'s entire output is a function of the clock (was regex-only tested); `_wsParseRetryAfter`'s HTTP-date branch had a demonstrated defect — a loose range assertion in place of an exact one, because the test couldn't pin `Date.now()`. |
|
||||
| `init.cts` | Backfilled | `cmdInitManager`'s `is_active` gate (`nowMs - newestMtime < 300000`) had zero boundary coverage; `cmdInitQuick`'s `quick_id` generation already used `realClock` for two of its three time-derived output fields but not the third, and its own test admitted non-determinism ("we just verify format"). |
|
||||
| `io.cts` | Backfilled | `reapStaleTempFiles`'s `maxAgeMs` gate had no `limit-1`/`limit`/`limit+1` boundary coverage; already in-process reachable by `t.mock.timers`, so no production code change was needed, only tests. |
|
||||
| `roadmap.cts`, `workstream.cts`, `gsd2-import.cts`, `template.cts`, `verify.cts` | Incidental — no change | Each calls `realClock.localToday()`/`nowIso()` only to **stamp** a date into written content (`created:`, `Last Activity:`, archive-dir suffixes, backup filenames, a "(Backfilled: <date>)" note) — no branch, comparison, or gating decision depends on the value. Already routed through the sanctioned seam; adding exact-value tests here would assert "today equals today," not exercise real logic. |
|
||||
| `review-lane-invocation.cts` | Correction — does not touch the clock | Named in the epic's original module list, but the file's own header states "PURE. No filesystem, no network, no subprocess, no clock," confirmed by zero `Date`/`clock` references in the file. Likely refactored to pure-function shape after the epic was drafted. |
|
||||
| `clock.cts` | N/A — is the seam itself | Already has dedicated coverage (`tests/clock-seam.test.cjs`, `tests/fix-2136-clock-local-today.test.cjs`); it is the thing being tested against, not a consumer needing backfill. |
|
||||
|
||||
### (b) Antagonistic tier — property-based and mutation testing
|
||||
|
||||
Two tools form the antagonistic tier:
|
||||
|
||||
Reference in New Issue
Block a user