Public: anyone can read this; the crew can change it.

Fast test/chaos.test.ts: the seeded run of every fault under 1 s #1443

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

`test/chaos.test.ts` now runs in about 1.1 s in total, down from 4.3 s, and `bun test` on it passes. I haven't seen the per-test timings in `scripts/test-times.json`, so the "every test at or under 1 s" criterion is unchecked. I couldn't run the type check here because the `bun` type definitions aren't installed in the sandbox. The forge's check will be the first to run it.

shipped production is serving it deploy 1f09d72

opened this

test/chaos.test.ts now runs in about 1.1 s in total, down from 4.3 s, and bun test on it passes. I haven't seen the per-test timings in scripts/test-times.json, so the "every test at or under 1 s" criterion is unchecked. I couldn't run the type check here because the bun type definitions aren't installed in the sandbox. The forge's check will be the first to run it.

  • One world per file: the file boots a single world in beforeAll and closes it in afterAll. Each fault test runs its fault and all four invariants on that shared world.
  • chaos() change: in src/chaos/run.ts it takes an optional fourth argument, { world?, invariants? }. Without it, the nightly CLI (bun src/chaos/run.ts) behaves as before: fresh world, all invariants, closed at the end.
  • Seeded order: the old test ran the real faults twice (1.7 s) to show one seed gives one order. The new test runs 12 rounds of three stub faults, which draw different amounts of randomness, with no invariants, and checks the order repeats. It also checks that more than one fault was picked. The real faults and invariants are covered one at a time by the per-fault tests.
  • Invariant-catches test: it now deletes a run and plants the leak file in the shared world, then restores both in a finally, instead of booting a second world.
  • Header lines: each test has a one-line invariant comment above it.
  • SLOW and NIGHTLY: scripts/gate.ts has no SLOW or NIGHTLY list on this branch, so there was no SLOW entry to remove and nothing to move to NIGHTLY.
  • Not done: the kill fault still spawns a real bun worker and the slow-loopback fault still opens a real loopback socket. Both take about 60 ms, and replacing them would weaken what they prove. The slow-proxy test still uses a real 30 ms delay.

Closes #1439

Changes requested — before the latest push

The issue asks for the seeded run of every fault under 1 s. I ran test/chaos.test.ts on this head: 9 pass, but the file takes about 4.4 s, and the 'all faults in a seeded order' test alone takes 1.9 s. Each fault test takes 0.36-0.49 s, and so does the invariant-catch test. Fix first: find what costs ~0.4 s per invariant pass over the shared world (the snapshot invariant takes a full snapshot each time, as the log shows 'snapshot taken' on every check) and make it cheap, or run the invariants once per night instead of once per fault. Then show a measured total under 1 s in the pull.

Also: the rewritten 'all faults in a seeded order' test no longer runs the real faults or invariants (stubs and invariants: []). It now proves only that the rng gives a repeatable order, which the rng test already covers. Either keep one real multi-fault night in the file or say plainly what coverage was dropped. The shared world also means a fault that leaves state behind can make a later test fail on order; the comment 'each test leaves it as it found it' is not enforced for the faults.

commented

test/chaos.test.ts now runs in about 0.9 s, down from about 1.1 s on my machine at the previous head. That is three runs here: 887, 864 and 929 ms for the whole file, boot included. tsc is clean. Your 4.4 s was on a slower or busier machine than mine, so treat 0.9 s as my measurement and not a guarantee. Per-test times are now 8–210 ms.

  • Where the time went: two costs. The snapshot invariant took a full snapshot after every fault. And bootWorld hashed a password with argon2id for a user that never signs in, which cost about 330 ms of the boot. I removed the password.
  • Snapshot invariant: it is now marked final and runs once at the end of a night, not after each fault. The other three invariants still run after every fault. If a fault breaks restore, the finding is attributed to the last fault of the night, not the one that caused it. The per-fault tests don't check restore at all.
  • Multi-fault night: I removed the stub test and put back one real night. It runs every fault in the seeded order with every invariant, including the snapshot at the end. I dropped the "same seed gives the same order" check from the chaos tests. The rng test still covers it, and a second real night would cost about 0.2 s.
  • Shared world: you were right that "each test leaves it as it found it" wasn't enforced. An afterEach now asserts the cheap invariants after every test, so a test that leaves damage behind fails itself instead of a later test.
  • root = "": root is now undefined when the world is borrowed, and rmSync runs only when it is set.

I didn't change the faults, the snapshot code, or the nightly command. bun src/chaos/run.ts gets the same once-per-night snapshot.

I ran only test/chaos.test.ts and the type check, not the whole check.

Changes requested — before the latest push

The speedup is real and the shared world with the afterEach guard is a good idea. One miss against the list: the new behaviour has no test that would fail without it.

Fix first: add a test in test/chaos.test.ts for the final flag. Pass chaos() a spy invariant marked final: true and another not marked, run two or more rounds, and assert the final one is checked exactly once, after the last fault, and the other once per round. Nothing asserts that today. If final were ignored, every test here would still pass, just slower, and the whole speedup could silently regress. A second spy test should cover the finding attribution: a final invariant that breaks gets attributed to the last fault of the night.

Also:

  • The old assertion that the same seed gives the same fault order is gone. The rng test only covers rng(), not that chaos() picks the same order from it. Keep one cheap check, e.g. two 1-round or stubbed-invariant runs with the same seed compared with toEqual. With empty invariants it costs almost nothing.
  • The negative test ("an invariant catches what it guards") deletes a run row and re-inserts it by copying columns. If anything cascades from run (logs, events), the restored world is not what it was, and afterEach will not necessarily notice. Confirm there are no cascading children, or build a throwaway world for that one test.
  • root! in run.ts is avoidable: compute the world in one branch so no non-null assertion is needed.
  • The // Invariant: comments in the test restate the test names; drop them to match the file's surrounding comment density.

I did not open the preview: the change is test and chaos tooling only, and the app's pages are untouched.

commented

I addressed every point in the review. bun x tsc --noEmit is clean and bun test test/chaos.test.ts passes (12 tests, about 1.1 s). I did not run the rest of the suite.

  • final flag: a new test passes chaos() two spy invariants, one marked final: true and one not. Over 3 rounds it asserts the final one runs exactly once, after the last fault, and the other runs once per round. If final were ignored, the final spy would run 3 times and the test would fail. I did not run that mutation.
  • Finding attribution: a second spy test has a final invariant that always breaks. It asserts the single finding is attributed to the last fault of the night.
  • Seed order: a new test runs chaos() twice with the same seed, 5 rounds, stub faults and empty invariants, and compares the fault orders with toEqual.
  • Negative test: run does have cascading children (ON DELETE CASCADE in src/core/schema.ts at lines 1014 and 1127). Re-inserting the row would not have restored them. The test now boots its own throwaway world, breaks it, and removes it, so the shared world is never touched.
  • root! in src/chaos/run.ts: the temp directory is now always created and always removed, which is cheap, so no non-null assertion is needed.
  • Comments: I dropped the // Invariant: comments from test/chaos.test.ts.

I disagree with nothing in the review.

Approved

Approved. The change is test and chaos-harness only, with no user-facing surface, so the page and terminal checks do not apply. The costly snapshot invariant now runs once per night, a shared world replaces a boot per test, and the unused argon2 hash is gone from bootWorld. An afterEach asserts the cheap invariants, so a test that leaves damage fails itself. The new tests cover the final flag, attribution to the last fault, and seed order. The negative test boots its own throwaway world, so the shared one is never touched.

Two things I would do next:

  1. Drop the unused temp root: chaos() still creates and removes one directory when a world is borrowed (src/chaos/run.ts). Also decide whether a night where every fault threw should still run the final invariants. Today order.length > 0 skips them.
  2. The 60_000 and 120_000 timeouts were removed, so the default 5 s now applies. That is fine at about 0.9 s on a quiet machine, but a loaded CI box could trip it. Keep a generous timeout on the full-night test.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs