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

Repo-wide rule tests (raw ms budgets, migration pins) skip a pull's own check, so rule breakers go green and sink merge batches (5 pulls today) #7143

Merged masoncrew opened this 17d agoin answer to trevatoowner

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

Merged

Every pull's check now runs the repo-wide rule tests, whatever the diff. A pull that breaks one goes red on its own check, with a summary that names the rule and the `file:line`, instead of passing and sinking a merge batch.

shipped production is serving it deploy eb43263

masoncrew opened this 17d ago

Every pull's check now runs the repo-wide rule tests, whatever the diff. A pull that breaks one goes red on its own check, with a summary that names the rule and the file:line, instead of passing and sinking a merge batch.

Cause. The pull check runs the "fast tier", picked by fastSet in src/checks/tiers.ts from coverage, imports, names and speed. Rule tests read the whole tree, so no diff ever selected them.

What changed

Always run (src/checks/tiers.ts): a new RULE_TESTS list joins every fast tier with the reason repo-wide rule. It holds test/flake.test.ts (raw ms budgets #4429, tight wall-clock bounds), test/schema-migrations.test.ts (migration pins), test/readme-front-door.test.ts and test/readme-cap.test.ts (the README budget) and test/design-audit.test.ts (the page audit). Locally flake and schema-migrations take about 1.2 s and 1.7 s.
Rule and line in the summary (test/helpers/rule.ts): a new breaches() helper makes one failing test per breach, named for the breach. The check summary already quotes failing test names, so it now reads like no raw wall-clock budget in a test (#4429) > test/x.test.ts:4: …. The raw-ms, tight-bound and migration-pin scans use it, and the raw-ms message also names #4429.
Test (test/rule-tests-always.test.ts):

- Every rule test is a gate file. - A diff of only test/x.test.ts runs every rule test, and so does a README-only diff. - A toBeLessThan(5) on a performance.now() delta, run by a real bun test, gives a summary that contains #4429 and test/x.test.ts:4:.

docs/DEVELOP.md and docs/OPERATING.md describe the always-run rule tests; a new tree-reading rule should be added to RULE_TESTS.

Worth a look

A pull that changes only docs now runs these five files instead of "nothing to run". That is intended, since the README budget can break on a README-only pull.
I limited the list to the rules this issue names. Other tests also scan the tree (src-imports, design-lint, openapi-coverage, no-polling); they can go on the list if they cause the same trouble.

Checks: the type check is clean, and the new test, flake, schema-migrations, check-tiers and check-summary tests pass (101). With the always-run line removed, the two fast-tier cases in the new test fail.

Try it: add a test file containing expect(performance.now() - t0).toBeLessThan(5) and run bun scripts/fast.ts --why. The five rule tests are listed as repo-wide rule, and bun test test/flake.test.ts fails, naming the rule and file:line.

🤖 Generated with Claude Code

Closes #7138

masoncrew commented 17d ago

Cause. The pull check runs the "fast tier", picked by fastSet in src/checks/tiers.ts from coverage, imports, names and speed. Rule tests read the whole tree, so no diff ever selected them.

What changed

Always run (src/checks/tiers.ts): a new RULE_TESTS list joins every fast tier with the reason repo-wide rule. It holds test/flake.test.ts (raw ms budgets #4429, tight wall-clock bounds), test/schema-migrations.test.ts (migration pins), test/readme-front-door.test.ts and test/readme-cap.test.ts (the README budget) and test/design-audit.test.ts (the page audit). Locally flake and schema-migrations take about 1.2 s and 1.7 s.
Rule and line in the summary (test/helpers/rule.ts): a new breaches() helper makes one failing test per breach, named for the breach. The check summary already quotes failing test names, so it now reads like no raw wall-clock budget in a test (#4429) > test/x.test.ts:4: …. The raw-ms, tight-bound and migration-pin scans use it, and the raw-ms message also names #4429.
Test (test/rule-tests-always.test.ts):

docs/DEVELOP.md and docs/OPERATING.md describe the always-run rule tests; a new tree-reading rule should be added to RULE_TESTS.

Worth a look

A pull that changes only docs now runs these five files instead of "nothing to run". That is intended, since the README budget can break on a README-only pull.
I limited the list to the rules this issue names. Other tests also scan the tree (src-imports, design-lint, openapi-coverage, no-polling); they can go on the list if they cause the same trouble.

🤖 Generated with Claude Code

masoncrew commented 17d ago

Every pull's check now runs the repo-wide rule tests, so a rule breaker goes red on its own check instead of in a merge batch. The check summary names the rule and the file:line. A diff of only the check's own timing or impact table still runs no tests.

Cause. The pull check runs the fast tier, which fastSet in src/checks/tiers.ts picks from coverage, imports and file names. The rule tests read the whole tree, so no diff ever selected them.

What changed

Always run: a new RULE_TESTS list in src/checks/tiers.ts joins every fast tier, with the reason repo-wide rule:

- test/flake.test.ts: raw ms budgets (#4429) and tight wall-clock bounds. - test/schema-migrations.test.ts: migration pins. - test/readme-front-door.test.ts and test/readme-cap.test.ts: the README budget. - test/design-audit.test.ts: the page audit.

Locally the two rule files that scan every test file or migration take about 1.2 s and 1.7 s.

The one exception: a diff of only test-times.json or test-impact.json can break no rule, so it still runs nothing and stays green. test/shards.test.ts (#3579) expects exactly that, and the first push broke it.
Rule and line in the summary: a new breaches() helper in test/helpers/rule.ts makes one failing test per breach, named after it. The check summary already quotes failing test names, so it reads no raw wall-clock budget in a test (#4429) > test/x.test.ts:4: …. The raw-ms, tight-bound and migration-pin scans use the helper, and the raw-ms message also names #4429.
Test (test/rule-tests-always.test.ts):

- Every rule test is in the gate. - A diff of only test/x.test.ts runs every rule test, and so does a README-only diff. - A diff of only the times table runs none. - A toBeLessThan(5) on a performance.now() delta, run by a real bun test, gives a summary containing #4429 and test/x.test.ts:4:. - With the always-run line removed, the two fast-tier cases fail.

docs/DEVELOP.md and docs/OPERATING.md describe the rule tests; a new rule that reads the whole tree goes on the list.

Worth a look: I listed only the rules this issue names. Other tests also scan the tree (src-imports, design-lint, openapi-coverage, no-polling); they can go on the list if they sink batches the same way.

Checks: the new test, test/shards.test.ts, test/check-tiers.test.ts and test/scripts-fast.test.ts pass, and the type check is clean. The other red shards in the failed log were stopped by fail-fast (exit 143), not failures of their own.

🤖 Generated with Claude Code

junipercrew Approved 17d ago

Approved. This is a small pipeline fix: five repo-wide rule tests now join every fast tier. Each breach is a red test named path:line, and the times and impact tables stay the one exception. The new test fails without the always-run line. The preview answers 200 on /. No page, security or tty surface changed. Two things to do next: (1) add src-imports, design-lint, openapi-coverage and no-polling to RULE_TESTS if they sink a batch the same way, and measure the added fast-tier seconds against FAST_BUDGET_SECONDS. (2) Tidy the over-long comment lines in src/checks/tiers.ts, which were edited in place and now wrap unevenly.

masoncrew commented 17d ago

Every pull's check now runs the repo-wide rule tests, whatever the diff. A pull that breaks one of these rules now fails its own check, and the summary names the rule and the file:line. Before this, such a pull passed its check and then failed a whole merge batch.

Cause. A pull's check runs only the "fast tier", which fastSet in src/checks/tiers.ts picks from coverage, imports, names and speed. The rule tests read the whole tree, so no diff ever selected them.

What changed

Always run (src/checks/tiers.ts): a new RULE_TESTS list is added to every fast tier with the reason repo-wide rule. It holds:

Exception: a diff that touches only the check's own times and impact tables still runs nothing, as before (#3579). test/shards.test.ts pins this.
Rule and line in the summary (test/helpers/rule.ts): a new breaches() helper turns each breach into its own failing test, named after the breach. The check summary already quotes failing test names, so it now reads like no raw wall-clock budget in a test (#4429) > test/x.test.ts:4: …. The raw-ms, tight-bound and migration-pin scans use it.
Test (test/rule-tests-always.test.ts): a diff that adds only test/x.test.ts gets every rule test in its fast plan. A toBeLessThan(5) on a performance.now() delta then produces a red test whose name includes #4429 and the line.
Docs: docs/DEVELOP.md and docs/OPERATING.md describe the always-run rule tests.

What to look at: RULE_TESTS and where it joins the plan in src/checks/tiers.ts, and the breaches() helper. On top of current main (cdb4d361), the type check passes, and the four covering test files pass (128 tests, 0 failures). The branch has no conflicts with main.

Not done here: the latest message on #7138 is an automatic triage note saying the issue waits behind the Later gate until it names a pillar. It asks for no code change. Choosing the pillar is a decision for a person, so I left the issue's labels as they are.

To try it: bun test test/rule-tests-always.test.ts test/flake.test.ts test/schema-migrations.test.ts test/shards.test.ts

🤖 Generated with Claude Code