fix(tests): derive time from the clock, not the calendar; retire four stale xfails (#2243) - #2244
Conversation
Full-suite sweep result — AC5, with the residual namedBoth legs finished (~54 min each, clean
The 23 on dev decompose exactly
The 8 is exactly the set this workflow's own JUnit artifacts report on The +1y leg: 15 expected, plus 4 I did not predictAll four are the instrument, not the repo, one root cause, and it breaks in both directions — the harness shifts the process clock but not the filesystem's:
Those four tests are correct. Recorded in So, honestly
Earlier in this PR I also reported the naive-swap run's 74 failures; ~30 of those were a PyJWT |
… stale xfails (#2243) Seventeen tests were permanently red on `dev` for reasons that have nothing to do with the product: two sets asserted against literal calendar strings, and a third documented a bug that has since been fixed. A suite with a standing red block trains people to read past it, which is what #2080 exists to stop. Hard-coded time buckets (5 failures). `test_ent96_timeline_split.py` asserted against the literal hour `"2026-08-14T09"`. `_gap_fill` builds a continuous UTC axis relative to `datetime.now(timezone.utc)`, so once that hour fell out of the window the bucket ceased to exist — `KeyError` on the dict lookups and `StopIteration` from the `next(...)` ones. The key is now derived from the same clock the axis is, at an offset kept strictly inside the narrowest window under test, so an hour boundary ticking mid-test cannot move it out. Hard-coded `started_at` (13 PostgreSQL failures, not the 9 in the issue — the count grows with the calendar). `insert_execution` defaulted to `2026-01-01T00:00:00.000000Z`, and the terminal write computes `duration_ms = now − started_at`: a fixed anchor gains ~31.5 billion ms a year and had already passed int32, raising `NumericValueOutOfRange` on PostgreSQL while SQLite quietly widened it. The default is now a few minutes ago, and A7's four timestamp-format variants are rendered from one recent instant — the formats were the point of that case, the calendar date was not. A7 also gained a bound on the duration, so a form parsed with the wrong offset shows up as ±5h instead of passing. `started_at=None` keeps meaning SQL NULL: A5b passes it deliberately to prove the NOT NULL constraint, so "caller said nothing" needed a sentinel — a `None` default would have turned that probe into a passing insert. Stale strict-xfails (3 failures, and a fourth found). The three backslash params reported `XPASS(strict)` because #2017 (PR #2024) fixed the `re.sub`-replacement bug they pinned. Retired to plain regression tests that assert the ROUND TRIP, not merely "does not raise" — not raising was the crash symptom; writing a backslash the agent decodes back to the same token is the contract. That last change required fixing the oracle, which exposed the fourth. This file's `agent_reads` was a hand-written replica that only stripped quotes, but #2023 made the encoding reversible (escape the escape character, then the quote; `unquote_env_value` reverses both in one scan). So value decoding now delegates to the REAL agent-side reader, loaded by path via the idiom `test_2023_env_quote_round_trip.py` already uses, and the line scan stays local because last-wins is what makes a duplicate PAT line dangerous (#2016). Against the real reader, `test_a_quote_in_the_token_round_trips` XPASSes: it had been reporting XFAIL — green — for a gap #2023 closed, and only the replica made the claim look true. Same class as the three above, reached from the other side: a stale marker hidden by a stale copy rather than by an XPASS. Measured, then retired. `tests/clockshift.py` is the discriminator this class needs. Grep finds 124 files under `tests/` carrying literal ISO timestamps and cannot tell an inert fixture value from one compared against now; running the clock forward can. Opt-in (`PYTHONPATH=tests pytest tests/unit -p clockshift`), `CLOCKSHIFT_DAYS`-tunable, no new dependency, and honest about its limits — it moves `datetime.now/utcnow/today` and `time.time`, not a database's own clock. Verified: ent#96 14 passed today AND at +365d; `test_1771c` 86 passed (43 SQLite + 43 PostgreSQL against a throwaway postgres:16-alpine, from 13 PG failures reproduced on unmodified dev first); PAT properties + the #2023 parity suite 97 passed with zero xfail/xpass; the fixed files green at both +365d and -400d. Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`datetime` is a C type with immutable attributes, so shifting the clock means installing a subclass — and that splits the class identity. A library imported AFTER the swap type-checks against the subclass, while a value produced by code that bound `datetime` BEFORE it is an instance of the original. PyJWT does exactly that (`isinstance(payload["exp"], datetime)` guards the datetime→epoch conversion in `api_jwt.encode`), so the naive swap failed ~30 auth/session tests with `TypeError: Object of type datetime is not JSON serializable`. That matters because of what this harness is FOR. Its output is meant to be read as a verdict on whether the suite is date-independent; a failure list mixing real calendar coupling with artifacts of the instrument is not a verdict, it is homework. Measured: the five files in that cluster (ent#281, ent#375, ent#163, ent#79, websocket auth) go from ~30 failures to 141 passed with no change to the tests themselves. Handled with a metaclass whose `__instancecheck__`/`__subclasscheck__` accept the real `datetime` too, so both identities satisfy such a check, and documented in the module docstring beside the limits that are NOT handled (a database's own clock, and any real-clock source like a filesystem mtime). Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The full-suite run at +365 days came back with four failures beyond the environment-specific set, and all four are the instrument rather than the repo: the harness moves the process clock but not the filesystem's, so a test that writes a file NOW and lets the code judge its age breaks — in both directions. `test_fresh_locks_kept` and `TestTmpSweep` see a fresh file as a year old and reap it; the two `test_prune_runs_*` cases see an out-of-window artifact as freshly created and correctly decline to prune it. Those tests are right and the shift cannot see their clock. Naming them, with the direction of each failure, is the difference between a sweep whose output is a verdict and one that hands the next reader four false leads. Shifting inode times would mean intercepting os.stat — a much bigger instrument than the problem justifies, and it would make the harness itself the thing under suspicion. Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bcedefe to
a2bc10e
Compare
…on the spot (#2247) Five ent#96 tests were red on `dev` from 2026-08-15 with no red check anywhere. The fixtures are #2243's half (PR #2244, which this branch is stacked on); this is the half the issue calls more interesting — WHY nothing said so, and what would have caught it. TWO GAPS, BOTH STRUCTURAL. `backend-unit-test.yml` runs pytest with `|| true` and gates on a base-vs-head diff: when both sides fail identically the diff is empty and the check is green. And on a push to dev/main the `test` and `diff` jobs are both `if: github.event_name != 'push'`, so nothing evaluates the suite at all. A test can therefore rot into permanent red without a single red check — invisible on PRs by construction, invisible on pushes by construction. WHAT THE NUMBERS ACTUALLY ARE. Rather than guess, I pulled the JUnit artifacts from a real run of this workflow: `dev` fails **exactly 8** tests under all three seeds — the 5 ent#96 plus the 3 stale strict-xfails — out of 35,229 testcases, and the #2243 branch takes it to **0**. So the `|| true` comment citing "34 failures + 17 errors documented in #660" was years of drift, and is corrected here. That measurement changes the design the issue proposed. With the suite reachable at zero, the cheapest gate that cannot itself rot is "no failures at all", not a count: `tests/unit_failure_baseline.txt` ships EMPTY. The ratchet mechanism exists (precedent: `lint_sys_modules_baseline.txt`) because a genuinely-expected failure should be tolerable deliberately — but a tolerated-failure list is where debt hides, so it starts empty, every entry has to be argued for in review, and the gate reports entries that have started passing so the file shrinks rather than grows. `scripts/ci/check-failure-baseline.py` + a new push-only `absolute` job: shares test identity with `diff-pytest-failures.py` (so a test cannot be baselined under one spelling and reported under another), distinguishes failure from collection error, unions across runs, and fails CLOSED on a missing/unparseable/ empty JUnit — #715's lesson that an infrastructure failure reporting green is the failure mode the gate exists to prevent. `--self-test` covers all nine of those behaviours and the workflow runs it before trusting the verdict. One run in default order, not the 3-seed matrix: seeds catch order-dependent regressions BETWEEN commits, which stays a PR-side property; tripling every dev push buys nothing and this job should stay cheap enough that nobody deletes it. Verified against the real artifacts, both directions: the gate fails on dev naming exactly those 8 ids, and passes on the fixed branch. DETECTING IT AT THE SOURCE. `tests/unit/test_2247_calendar_rot_detector.py` re-runs the calendar-sensitive files under `tests/clockshift.py` at +7 days and -365 days, in the ordinary unit suite, so a fixture pinned to a literal date inside a relative window fails on the first run instead of on a future Tuesday. A subprocess because the shift must land before the module binds `from datetime import datetime`. It also guards its own no-op failure mode — it asserts the plugin exists AND that importing it actually moves `datetime.now()`, since a vanished plugin would make every shifted run pass by shifting nothing. Proven to detect rather than merely to pass: reverting the ent#96 fixture to its pre-#2243 state turns both directions red with the message naming the cause, and restoring it turns them green. ~37s, nearly all pytest boot, which is why the file list is short and excludes the ~176s analytics-edges file. Python pinned to 3.13 to match the other jobs — deliberately, not by copy: the observed failing set is interpreter-dependent (on 3.12 locally the SSRF/IPv6 suites fail and on CI's 3.13 they do not), so a gate on a different Python would judge against a baseline collected under different rules. Stacked on #2244 (#2243): that PR owns the fixture fix and ships `clockshift.py`, which the detector here consumes. Merge it first. Related to #2247 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr.
Test-only. Replaces calendar-literal fixtures with clock-derived ones and retires four stale strict-xfails whose underlying bugs were fixed by #2017/#2024 — the three backslash cases now assert the round trip rather than merely 'does not raise', which is the actual contract. Ships tests/clockshift.py, the harness #2249 builds on.
…on the spot (#2247) (#2249) * fix(tests): derive time from the clock, not the calendar; retire four stale xfails (#2243) Seventeen tests were permanently red on `dev` for reasons that have nothing to do with the product: two sets asserted against literal calendar strings, and a third documented a bug that has since been fixed. A suite with a standing red block trains people to read past it, which is what #2080 exists to stop. Hard-coded time buckets (5 failures). `test_ent96_timeline_split.py` asserted against the literal hour `"2026-08-14T09"`. `_gap_fill` builds a continuous UTC axis relative to `datetime.now(timezone.utc)`, so once that hour fell out of the window the bucket ceased to exist — `KeyError` on the dict lookups and `StopIteration` from the `next(...)` ones. The key is now derived from the same clock the axis is, at an offset kept strictly inside the narrowest window under test, so an hour boundary ticking mid-test cannot move it out. Hard-coded `started_at` (13 PostgreSQL failures, not the 9 in the issue — the count grows with the calendar). `insert_execution` defaulted to `2026-01-01T00:00:00.000000Z`, and the terminal write computes `duration_ms = now − started_at`: a fixed anchor gains ~31.5 billion ms a year and had already passed int32, raising `NumericValueOutOfRange` on PostgreSQL while SQLite quietly widened it. The default is now a few minutes ago, and A7's four timestamp-format variants are rendered from one recent instant — the formats were the point of that case, the calendar date was not. A7 also gained a bound on the duration, so a form parsed with the wrong offset shows up as ±5h instead of passing. `started_at=None` keeps meaning SQL NULL: A5b passes it deliberately to prove the NOT NULL constraint, so "caller said nothing" needed a sentinel — a `None` default would have turned that probe into a passing insert. Stale strict-xfails (3 failures, and a fourth found). The three backslash params reported `XPASS(strict)` because #2017 (PR #2024) fixed the `re.sub`-replacement bug they pinned. Retired to plain regression tests that assert the ROUND TRIP, not merely "does not raise" — not raising was the crash symptom; writing a backslash the agent decodes back to the same token is the contract. That last change required fixing the oracle, which exposed the fourth. This file's `agent_reads` was a hand-written replica that only stripped quotes, but #2023 made the encoding reversible (escape the escape character, then the quote; `unquote_env_value` reverses both in one scan). So value decoding now delegates to the REAL agent-side reader, loaded by path via the idiom `test_2023_env_quote_round_trip.py` already uses, and the line scan stays local because last-wins is what makes a duplicate PAT line dangerous (#2016). Against the real reader, `test_a_quote_in_the_token_round_trips` XPASSes: it had been reporting XFAIL — green — for a gap #2023 closed, and only the replica made the claim look true. Same class as the three above, reached from the other side: a stale marker hidden by a stale copy rather than by an XPASS. Measured, then retired. `tests/clockshift.py` is the discriminator this class needs. Grep finds 124 files under `tests/` carrying literal ISO timestamps and cannot tell an inert fixture value from one compared against now; running the clock forward can. Opt-in (`PYTHONPATH=tests pytest tests/unit -p clockshift`), `CLOCKSHIFT_DAYS`-tunable, no new dependency, and honest about its limits — it moves `datetime.now/utcnow/today` and `time.time`, not a database's own clock. Verified: ent#96 14 passed today AND at +365d; `test_1771c` 86 passed (43 SQLite + 43 PostgreSQL against a throwaway postgres:16-alpine, from 13 PG failures reproduced on unmodified dev first); PAT properties + the #2023 parity suite 97 passed with zero xfail/xpass; the fixed files green at both +365d and -400d. Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tests): make the clockshift harness's type identity honest (#2243) `datetime` is a C type with immutable attributes, so shifting the clock means installing a subclass — and that splits the class identity. A library imported AFTER the swap type-checks against the subclass, while a value produced by code that bound `datetime` BEFORE it is an instance of the original. PyJWT does exactly that (`isinstance(payload["exp"], datetime)` guards the datetime→epoch conversion in `api_jwt.encode`), so the naive swap failed ~30 auth/session tests with `TypeError: Object of type datetime is not JSON serializable`. That matters because of what this harness is FOR. Its output is meant to be read as a verdict on whether the suite is date-independent; a failure list mixing real calendar coupling with artifacts of the instrument is not a verdict, it is homework. Measured: the five files in that cluster (ent#281, ent#375, ent#163, ent#79, websocket auth) go from ~30 failures to 141 passed with no change to the tests themselves. Handled with a metaclass whose `__instancecheck__`/`__subclasscheck__` accept the real `datetime` too, so both identities satisfy such a check, and documented in the module docstring beside the limits that are NOT handled (a database's own clock, and any real-clock source like a filesystem mtime). Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): record what the shifted clock cannot see (#2243) The full-suite run at +365 days came back with four failures beyond the environment-specific set, and all four are the instrument rather than the repo: the harness moves the process clock but not the filesystem's, so a test that writes a file NOW and lets the code judge its age breaks — in both directions. `test_fresh_locks_kept` and `TestTmpSweep` see a fresh file as a year old and reap it; the two `test_prune_runs_*` cases see an out-of-window artifact as freshly created and correctly decline to prune it. Those tests are right and the shift cannot see their clock. Naming them, with the direction of each failure, is the difference between a sweep whose output is a verdict and one that hands the next reader four false leads. Shifting inode times would mean intercepting os.stat — a much bigger instrument than the problem justifies, and it would make the harness itself the thing under suspicion. Related to #2243 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ci): make a wholly-red suite reportable, and detect calendar rot on the spot (#2247) Five ent#96 tests were red on `dev` from 2026-08-15 with no red check anywhere. The fixtures are #2243's half (PR #2244, which this branch is stacked on); this is the half the issue calls more interesting — WHY nothing said so, and what would have caught it. TWO GAPS, BOTH STRUCTURAL. `backend-unit-test.yml` runs pytest with `|| true` and gates on a base-vs-head diff: when both sides fail identically the diff is empty and the check is green. And on a push to dev/main the `test` and `diff` jobs are both `if: github.event_name != 'push'`, so nothing evaluates the suite at all. A test can therefore rot into permanent red without a single red check — invisible on PRs by construction, invisible on pushes by construction. WHAT THE NUMBERS ACTUALLY ARE. Rather than guess, I pulled the JUnit artifacts from a real run of this workflow: `dev` fails **exactly 8** tests under all three seeds — the 5 ent#96 plus the 3 stale strict-xfails — out of 35,229 testcases, and the #2243 branch takes it to **0**. So the `|| true` comment citing "34 failures + 17 errors documented in #660" was years of drift, and is corrected here. That measurement changes the design the issue proposed. With the suite reachable at zero, the cheapest gate that cannot itself rot is "no failures at all", not a count: `tests/unit_failure_baseline.txt` ships EMPTY. The ratchet mechanism exists (precedent: `lint_sys_modules_baseline.txt`) because a genuinely-expected failure should be tolerable deliberately — but a tolerated-failure list is where debt hides, so it starts empty, every entry has to be argued for in review, and the gate reports entries that have started passing so the file shrinks rather than grows. `scripts/ci/check-failure-baseline.py` + a new push-only `absolute` job: shares test identity with `diff-pytest-failures.py` (so a test cannot be baselined under one spelling and reported under another), distinguishes failure from collection error, unions across runs, and fails CLOSED on a missing/unparseable/ empty JUnit — #715's lesson that an infrastructure failure reporting green is the failure mode the gate exists to prevent. `--self-test` covers all nine of those behaviours and the workflow runs it before trusting the verdict. One run in default order, not the 3-seed matrix: seeds catch order-dependent regressions BETWEEN commits, which stays a PR-side property; tripling every dev push buys nothing and this job should stay cheap enough that nobody deletes it. Verified against the real artifacts, both directions: the gate fails on dev naming exactly those 8 ids, and passes on the fixed branch. DETECTING IT AT THE SOURCE. `tests/unit/test_2247_calendar_rot_detector.py` re-runs the calendar-sensitive files under `tests/clockshift.py` at +7 days and -365 days, in the ordinary unit suite, so a fixture pinned to a literal date inside a relative window fails on the first run instead of on a future Tuesday. A subprocess because the shift must land before the module binds `from datetime import datetime`. It also guards its own no-op failure mode — it asserts the plugin exists AND that importing it actually moves `datetime.now()`, since a vanished plugin would make every shifted run pass by shifting nothing. Proven to detect rather than merely to pass: reverting the ent#96 fixture to its pre-#2243 state turns both directions red with the message naming the cause, and restoring it turns them green. ~37s, nearly all pytest boot, which is why the file list is short and excludes the ~176s analytics-edges file. Python pinned to 3.13 to match the other jobs — deliberately, not by copy: the observed failing set is interpreter-dependent (on 3.12 locally the SSRF/IPv6 suites fail and on CI's 3.13 they do not), so a gate on a different Python would judge against a baseline collected under different rules. Stacked on #2244 (#2243): that PR owns the fixture fix and ships `clockshift.py`, which the detector here consumes. Merge it first. Related to #2247 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: trinity-ability <trinity@ability.ai>
What this fixes
17 tests were permanently red on
devfor reasons unrelated to the product. All three sets are fixed, and a fourth stale marker turned up while fixing the third.1. Hard-coded time buckets — 5 failures
test_ent96_timeline_split.pyasserted against the literal hour"2026-08-14T09"._gap_fillbuilds a continuous UTC axis relative todatetime.now(timezone.utc), so once that hour left the window the bucket stopped existing —KeyErroron the dict lookups,StopIterationon thenext(...)ones.The key is now derived from the same clock the axis is, at an offset kept strictly inside the narrowest window under test (6h), so an hour boundary ticking between the test's
nowand_gap_fill's cannot move it out. The row variable was renamedcolso nothing depends on a rebind inside a generator expression.2. Hard-coded
started_at— 13 PostgreSQL failuresNot the 9 the issue lists: the count grows with the calendar, and I reproduced 13 on unmodified
devbefore changing anything.insert_executiondefaulted to2026-01-01T00:00:00.000000Z. The terminal write computesduration_ms = now − started_at, so a fixed anchor gains ~31.5 billion ms a year; it had already passed int32 and raisedNumericValueOutOfRangeon PostgreSQL, while SQLite — untyped — kept quietly widening the value.Z/+00:00/+05:00) but renders them from one recent instant: the formats were the point of that case, the calendar date was not. It also gained a bound on the resulting duration, so a form parsed with the wrong offset now fails loudly (±5h) instead of passingstarted_at=Nonestill means SQL NULL. A5b passes it deliberately to prove theNOT NULLconstraint is what stands between production andparse_iso_timestamp(None), so "caller said nothing" needed a sentinel — aNonedefault silently turned that probe into a passing insert (the suite caught this mid-change)3. Stale strict-xfails — 3 failures, and a fourth found
The three backslash params reported
XPASS(strict)because #2017 (PR #2024) fixed there.sub-replacement bug they pinned. Retired to plain regression tests that assert the round trip rather than merely "does not raise" — not raising was the crash symptom; writing a backslash the agent decodes back to the same token is the contract.That required fixing the oracle, which exposed the fourth.
agent_readsin this file was a hand-written replica that only stripped quotes — but #2023 made the.envencoding reversible (escape the escape character, then the quote;unquote_env_valuereverses both in one scan). So:test_2023_env_quote_round_trip.pyalready uses; the line scan stays local because last-wins is what makes a duplicate PAT line dangerous (bug(github-pat): a duplicated GITHUB_PAT line survives rotation and wins — the agent keeps the revoked token while the rotation reports success #2016)test_a_quote_in_the_token_round_tripsXPASSes. It had been reporting XFAIL — green — for a gap bug(credentials): .env quote-escaping is write-only — a credential containing a double quote round-trips corrupted #2023 closed, and only the replica made the claim look true. Same class as the three above reached from the other side: a stale marker hidden by a stale copy rather than by an XPASS. Measured (ghp_a"bround-trips exactly), then retired.4. Closing the class, not patching it twice —
tests/clockshift.pyGrep finds 124 files under
tests/carrying literal ISO timestamps, and no grep can tell an inert fixture value from one compared against now. Running the clock forward can.Opt-in pytest plugin,
CLOCKSHIFT_DAYS-tunable (negative works too), no new test dependency — the issue asked to prefer deriving over addingfreezegun, so this shifts only what test code reads (datetime.now/utcnow/today,time.time) and documents its own limits: it does not move a database's own clock, so a test whose expectations come from SQL-side time is out of its reach. It is not aconfteston purpose — it must not load for runs that did not ask for it.Verification
test_ent96_timeline_split.py+365dtest_1771c_schedules_cas_edges.pydevfirstpostgres:16-alpine+alembic upgrade head, perrun-full.sh --tier postgrestest_2023parity+365dand at-400dThe full-unit
+1ysweep (the empirical answer to "any other calendar-dependent failures") is running against unmodifieddevas the baseline; I'll post the residual list as a comment.Not fixed, noted:
test_ent96_timeline_split.pyhas no entry intests/registry.json. Pre-existing and unrelated to this issue, so I left it rather than widen the diff.Fixes #2243
🤖 Generated with Claude Code