test(e2e): ratchet new unconditional sleeps to dwell/injectLatency (#2646)
Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
@@ -209,6 +209,36 @@ jobs:
|
||||
|
||||
node scripts/run-i18n-ratchet.mjs --base "${BASE_SHA}"
|
||||
|
||||
# New-code ratchet for E2E waits. `page.waitForTimeout(500)` and
|
||||
# hand-rolled promise sleeps are the suite's most reliable source of flake:
|
||||
# a hand-picked number that was long enough on the machine it was written
|
||||
# on. The sanctioned forms (`dwell`, `injectLatency` in
|
||||
# e2e/helpers/causal-waits.ts) take a category and a reason.
|
||||
#
|
||||
# The eslint rule is an error only on directories already converted to
|
||||
# zero, so this judges the CHANGE instead of the FILE for everywhere else —
|
||||
# added files whole, modified files only on touched lines. Same base
|
||||
# handling as the i18n step above, and the same reason for it: DO NOT pass
|
||||
# --base on pull_request.
|
||||
- name: Run E2E sleep ratchet
|
||||
shell: bash
|
||||
working-directory: apps/web
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
|
||||
node scripts/check-new-e2e-sleeps.mjs
|
||||
exit 0
|
||||
fi
|
||||
|
||||
BASE_SHA="${{ github.event.before }}"
|
||||
if [[ -z "${BASE_SHA}" || "${BASE_SHA}" == "0000000000000000000000000000000000000000" ]]; then
|
||||
echo "No usable base SHA; skipping the E2E sleep ratchet."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
node scripts/check-new-e2e-sleeps.mjs --base "${BASE_SHA}"
|
||||
|
||||
# Deliberately run under the variable that broke the suite, so a
|
||||
# regression fails here instead of only in a container. React exports
|
||||
# `act()` from its development build alone, `vitest.config.ts` pins
|
||||
|
||||
@@ -69,6 +69,19 @@ repos:
|
||||
files: ^apps/web/(components|app|hooks|lib|src)/.*\.tsx?$|^apps/web/eslint\.i18n\.options\.mjs$
|
||||
pass_filenames: false
|
||||
|
||||
# New E2E waits must name their cause, or name a category and a reason via
|
||||
# dwell()/injectLatency(). Raw page.waitForTimeout() and hand-rolled
|
||||
# promise sleeps are the suite's most reliable source of flake. Same
|
||||
# judge-the-change contract as the i18n hook above: added files whole,
|
||||
# modified files only on touched lines, so the ~126 sleeps that predate it
|
||||
# never block an unrelated commit.
|
||||
- id: e2e-sleep-ratchet
|
||||
name: E2E sleep guard (new code only)
|
||||
entry: bash -c 'unset GIT_DIR GIT_WORK_TREE; cd apps/web && node scripts/check-new-e2e-sleeps.mjs'
|
||||
language: system
|
||||
files: ^apps/web/e2e/.*\.tsx?$|^apps/web/eslint-rules/no-unsanctioned-sleep\.mjs$
|
||||
pass_filenames: false
|
||||
|
||||
# Public copy hooks
|
||||
- repo: local
|
||||
hooks:
|
||||
|
||||
@@ -444,6 +444,153 @@ A ratchet banning raw sleeps needs to allow **both** tokens, `dwell(` and
|
||||
instead — a proximity heuristic misfires, and a wrong exemption silently
|
||||
unguards real sleeps.
|
||||
|
||||
### How this is enforced
|
||||
|
||||
**The sanctioned forms are only sanctioned when they are actually imported.**
|
||||
`dwell` and `injectLatency` must resolve to a binding from
|
||||
`e2e/helpers/causal-waits`; a call to an unimported (or differently imported)
|
||||
identifier of that name is reported like a raw sleep. It is not pedantry: the
|
||||
repo turns `no-undef` off, nothing typechecks `e2e/`, and an import that
|
||||
silently fails to apply leaves a call site that lints clean and throws
|
||||
`dwell is not defined` at runtime, inside whichever retry path only executes on
|
||||
a loaded shard. That has already happened once.
|
||||
|
||||
Two checks, deliberately scoped differently, because ~126 raw sleeps predate
|
||||
them and `pnpm lint` is `eslint --max-warnings 0` — a repo-wide rule at _any_
|
||||
severity would break every unrelated PR until the conversion finishes.
|
||||
|
||||
**1. The new-code ratchet** (`scripts/check-new-e2e-sleeps.mjs`) runs in CI and
|
||||
pre-commit and judges the **change**, not the file:
|
||||
|
||||
- a file the change **added** must be clean outright;
|
||||
- a file the change **modified** is judged only on the lines it touched.
|
||||
|
||||
So editing a line next to somebody else's `waitForTimeout` never becomes your
|
||||
problem, and there is no migration treadmill. Same model as
|
||||
`golangci-lint --new-from-rev` and the i18n ratchet.
|
||||
|
||||
**2. The eslint rule** (`eslint-rules/no-unsanctioned-sleep.mjs`) is an
|
||||
**error**, but only on `e2eSleepGuardFiles` — directories the conversion has
|
||||
already driven to zero. Append a directory there in the same PR that clears it.
|
||||
**Never remove an entry to make a build pass**: a removal means a sleep was added
|
||||
to a clean directory.
|
||||
|
||||
When the list would cover all of `e2e/`, replace it with `e2e/**/*.ts`. That is
|
||||
the graduation, and from then on the ratchet is a second line of defence rather
|
||||
than the only one.
|
||||
|
||||
Two commands:
|
||||
|
||||
```bash
|
||||
pnpm run e2e:waits # debt report: raw sleeps, dwell by category
|
||||
pnpm run lint:e2e-sleeps <path> # preview the rule on a path not yet guarded
|
||||
```
|
||||
|
||||
`e2e:waits` reports three numbers that must not be added together. Raw sleeps are
|
||||
what the conversion drives to zero. **Dwell debt excludes `negative-assertion`,
|
||||
which is permanent** — a test asserting something never happens has no event to
|
||||
wait for, and deleting its wait removes the window the regression needs in order
|
||||
to appear. `injectLatency` is counted apart from both, so deliberate fixture
|
||||
configuration cannot inflate a number that is supposed to be shrinking. Inside
|
||||
the debt, `unverified` is the sub-total to attack first.
|
||||
|
||||
**What the rule does not flag, and why.** It is an AST rule rather than a grep
|
||||
because `e2e/` holds ~700 `test.setTimeout(60_000)` calls — Playwright's per-test
|
||||
timeout setter, unrelated to sleeping. It also leaves alone two shapes that look
|
||||
almost identical to a sleep:
|
||||
|
||||
- a **race guard**, where the timer is one of several resolution paths
|
||||
(`socket.onopen = …; setTimeout(() => resolve(false), 3000)`);
|
||||
- a **cancellable timer**, where the handle is kept for a later `clearTimeout`.
|
||||
A sleep never keeps the handle.
|
||||
|
||||
The same reasoning leaves `requestAnimationFrame(() => setTimeout(resolve, 0))`
|
||||
alone: that promise resolves on the **frame**, and the `setTimeout(…, 0)` only
|
||||
hops out of the rAF callback so a measurement reflects painted geometry. It is
|
||||
not a duration wait. This falls out of the rule's shape rather than from a
|
||||
carve-out — a blanket "any `setTimeout(…, 0)` is fine" exemption would have been
|
||||
a loophole.
|
||||
|
||||
## Verifying a converted wait
|
||||
|
||||
Replacing a sleep with a causal wait is easy to do wrongly in a way that still
|
||||
passes. These four checks are ordered by how often they actually catch
|
||||
something.
|
||||
|
||||
### 1. Verify against observed traffic, never a name that looks right
|
||||
|
||||
Reading the components and picking the action that sounds correct is the single
|
||||
most common way a conversion goes wrong. Attach a throwaway logger, run the spec
|
||||
once, and read what actually arrived:
|
||||
|
||||
```ts
|
||||
page.on("response", (r) => console.log(r.request().method(), new URL(r.url()).pathname));
|
||||
page.on("websocket", (ws) =>
|
||||
ws.on("framereceived", (f) => console.log(String(f.payload).slice(0, 200))),
|
||||
);
|
||||
```
|
||||
|
||||
Three conversions in this suite were wrong despite careful code-reading: a spec
|
||||
named after `kanban.update` whose path only ever delivers `task.updated`; a
|
||||
branch list fetched on the workspace-scoped route rather than the
|
||||
repository-scoped one the client's shape implies; and a `task.plan.get` that
|
||||
never fires on first panel open at all. Each looked right in the source.
|
||||
|
||||
### 2. Rule out the silent burn
|
||||
|
||||
A wait that resolves on timeout instead of throwing converts a hard failure into
|
||||
a slow pass — the test still goes green, having asserted nothing about timing.
|
||||
Every armed wait must be awaited, and must reject rather than resolve when it
|
||||
expires (the primitives in `helpers/causal-waits.ts` do).
|
||||
|
||||
Then compare **runtime against the budget**. A spec carrying two 15s waits that
|
||||
finishes in 6.3s cannot be burning either of them; one that finishes in 30.1s is
|
||||
burning both. One fixed conversion here went from a 15s burn to 6.8s, and that
|
||||
delta is what exposed it — not any assertion.
|
||||
|
||||
### 3. Run it enough times to mean something
|
||||
|
||||
```sh
|
||||
pnpm run build:e2e # specs run against the prebuilt dist
|
||||
pnpm e2e:raw tests/<area>/<spec>.spec.ts --repeat-each=10
|
||||
```
|
||||
|
||||
Read the **spread**, not the pass count. `1.4s-11.4s` across ten runs with zero
|
||||
failures is evidence the wait absorbs load; a single green 1.4s run is evidence
|
||||
of nothing. A component edit is invisible until `build:e2e` re-runs, so a spec
|
||||
that "passes after the fix" without a rebuild has told you nothing.
|
||||
|
||||
### 4. Mutate what you rely on
|
||||
|
||||
Break the thing the test depends on, confirm the intended test fails, restore,
|
||||
confirm the tree is byte-identical. Assert the expected number of tests was
|
||||
**collected and executed** — a mutant that fails to compile makes the runner
|
||||
report zero collected, which at a glance is indistinguishable from a clean run.
|
||||
|
||||
### Two traps
|
||||
|
||||
**Wrong Playwright project.** `mobile-*.spec.ts` exists only under
|
||||
`mobile-chrome`, `tests/auth/**` under `auth`, `office-routing-*` under
|
||||
`routing`, and `tests/{docker,ssh}/**` under `containers`. Selecting one under
|
||||
`chromium` matches nothing, prints `No tests found`, and **exits non-zero** — so
|
||||
the failure is real. What hides it is the idiom: `playwright test … | tail`
|
||||
reports `tail`'s exit status, not Playwright's. **Never pipe a gate.** Derive the
|
||||
expected test count before the run and compare it to Playwright's first line;
|
||||
checking afterwards is a habit, deriving first is a check.
|
||||
|
||||
**Nothing typechecks `e2e/**`.** `apps/web/tsconfig.json`excludes it, there is
|
||||
no`e2e/tsconfig.json`, and eslint runs no type-aware rules here — a file
|
||||
containing `const x: number = "string"`passes both`pnpm run typecheck`and`pnpm lint`. Anything load-bearing needs a runtime check or a test. `dwell`
|
||||
validates its category at runtime for exactly this reason.
|
||||
|
||||
### Why this matters
|
||||
|
||||
**A sleep is where a test goes to stop asserting.** Converting them has already
|
||||
found two specs that had quietly stopped testing anything. The clearest was a
|
||||
test pressing a keybinding that is `UNBOUND_SHORTCUT` by default: the keypress
|
||||
was a no-op, and the 250ms sleep after it made the test look like it was waiting
|
||||
for a result it could never receive.
|
||||
|
||||
## Adding a new spec
|
||||
|
||||
1. Pick a directory under `tests/` (or create one for a new feature).
|
||||
|
||||
@@ -0,0 +1,333 @@
|
||||
/**
|
||||
* `no-unsanctioned-sleep` — an unconditional wall-clock sleep in `e2e/` must go
|
||||
* through `dwell()` or `injectLatency()` from `e2e/helpers/causal-waits.ts`.
|
||||
*
|
||||
* Those two helpers demand a category and a reason. A raw sleep demands nothing,
|
||||
* which is why it is indistinguishable — at a glance and to a grep — from
|
||||
* somebody who could not find the right event and reached for a number instead.
|
||||
* The whole point of the causal-waits work is that the sanctioned forms are
|
||||
* greppable and self-documenting; that only holds if the unsanctioned ones
|
||||
* cannot be added silently.
|
||||
*
|
||||
* ## Why this is an AST rule and not a grep
|
||||
*
|
||||
* `e2e/` contains **701** `test.setTimeout(60_000)` calls — Playwright's
|
||||
* per-test timeout setter, which has nothing to do with sleeping and must never
|
||||
* be flagged — against ~38 real promise sleeps. Any `setTimeout` pattern that a
|
||||
* regex can express either drowns in those 701 or misses the multi-line sleeps.
|
||||
* It also cannot separate a sleep from a `Promise.race` timeout guard, which is
|
||||
* correct code that looks almost identical. The AST separates all three exactly.
|
||||
*
|
||||
* ## What is reported
|
||||
*
|
||||
* 1. **`waitForTimeout`**, in any receiver position. Playwright exposes it on
|
||||
* `Page`, `Frame` and `Locator.page()`, so the receiver is not fixed; the
|
||||
* method name is. `dwell(page, ms, category, reason)` is the legal form.
|
||||
*
|
||||
* 2. **The promise-sleep idiom** — `new Promise((r) => setTimeout(r, ms))` — in
|
||||
* the narrow shape where the timer is the promise's *only* resolution path.
|
||||
* `dwell(ms, category, reason)` is the legal form where no `Page` exists.
|
||||
*
|
||||
* ## What is deliberately NOT reported, and why the narrowing is principled
|
||||
*
|
||||
* The executor must contain **exactly one statement**, that statement's value
|
||||
* must be **discarded**, and its first argument must **resolve the promise**.
|
||||
* Each clause excludes a real, correct pattern in this suite:
|
||||
*
|
||||
* - *More than one statement* excludes a race guard. `auth-lifecycle.spec.ts`
|
||||
* registers `socket.onopen/onclose/onerror` and then a `setTimeout` fallback:
|
||||
* the timer is one of four resolution paths, not a sleep.
|
||||
* - *Value discarded* excludes a cancellable timer. `concurrency.spec.ts` has a
|
||||
* lone `timer = setTimeout(() => resolve({kind:"timeout"}), ms)` inside its
|
||||
* executor — one statement, but it keeps the handle because something else
|
||||
* will `clearTimeout` it. **A sleep never keeps the handle**; keeping it is
|
||||
* precisely the intent to cancel, which means another path resolves first.
|
||||
* - *First argument resolves the promise* keeps the diagnostic honest: it makes
|
||||
* the finding "this is a sleep" rather than merely "this has the right shape".
|
||||
* "Resolves the promise" includes bound references — `resolve.bind(null)` is
|
||||
* still `resolve`, and recursively so, since a form that laundered a sleep
|
||||
* past the rule would be worth more to a determined author than to an honest
|
||||
* one.
|
||||
*
|
||||
* The same three clauses dissolve the `requestAnimationFrame(() =>
|
||||
* setTimeout(resolve, 0))` frame-yield in `toggle-sidebar-shortcut.spec.ts`
|
||||
* without a carve-out: the executor's one statement is a `requestAnimationFrame`
|
||||
* call, not a `setTimeout` one. That is not a convenient exception — the promise
|
||||
* there resolves on the **frame**, and the `setTimeout(…, 0)` only hops out of
|
||||
* the rAF callback so the measurement reflects painted geometry. It is not a
|
||||
* duration wait, so a rule about duration waits should not see it. A blanket
|
||||
* "any `setTimeout(…, 0)` is fine" carve-out would have been a loophole; this
|
||||
* needs no carve-out at all.
|
||||
*
|
||||
* ## Severity
|
||||
*
|
||||
* An **error**, but only on {@link e2eSleepGuardFiles} — the directories already
|
||||
* converted to zero. Not a warning either: `pnpm lint` is
|
||||
* `eslint --max-warnings 0`, so a repo-wide registration at *any* severity would
|
||||
* fail the build on the ~126 sleeps that predate this rule and block every
|
||||
* unrelated PR, which is the mistake the first i18n attempt made.
|
||||
*
|
||||
* Everywhere else the gate is `scripts/check-new-e2e-sleeps.mjs`, which judges
|
||||
* only the lines a change added. Once the conversion reaches zero the guard list
|
||||
* collapses to `e2e/**\/*.{ts,tsx}` and the ratchet becomes belt-and-braces.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Files exempt from the rule, repo-relative from `apps/web`.
|
||||
*
|
||||
* `causal-waits.ts` implements `dwell` and `injectLatency` and therefore
|
||||
* contains both banned forms by construction — `dwell` delegates to
|
||||
* `page.waitForTimeout(ms)` when a page exists and to a promise sleep when one
|
||||
* does not. Without this the sanctioned helper is the one file that can never
|
||||
* satisfy the rule it exists to serve.
|
||||
*
|
||||
* Its own `*.test.ts` is exempt for the same reason: tests for a sleep helper
|
||||
* necessarily construct sleeps.
|
||||
*
|
||||
* Exported so `eslint.config.mjs` and `eslint.e2e-sleeps.config.mjs` cannot
|
||||
* drift — an exemption that applies in the editor but not in the gate (or the
|
||||
* reverse) is a difference nobody would notice until it fired.
|
||||
*/
|
||||
export const SLEEP_EXEMPT_FILES = [
|
||||
"e2e/helpers/causal-waits.ts",
|
||||
"e2e/helpers/causal-waits.test.ts",
|
||||
];
|
||||
|
||||
/**
|
||||
* Paths where this rule is an **error** in `eslint.config.mjs`.
|
||||
*
|
||||
* Same opt-in shape as `i18nGuardFiles`, and for the same reason: `pnpm lint` is
|
||||
* `eslint --max-warnings 0`, so a repo-wide registration at any severity fails
|
||||
* the build on the ~126 sleeps that predate the rule and blocks every unrelated
|
||||
* PR. Listing only directories already at zero means this can only ever tighten.
|
||||
*
|
||||
* The seed was measured, not guessed — `pnpm run e2e:waits` reports remaining
|
||||
* raw sleeps per file, and every directory below had none at the time it was
|
||||
* added. Adding a path is the only safe edit: **never remove one to make a build
|
||||
* pass.** A removal means somebody added a sleep to a directory that had none,
|
||||
* which is exactly the regression this exists to catch.
|
||||
*
|
||||
* The conversion effort is working through the directories that are *not* here
|
||||
* (`tests/{auth,chat,layout,office,session,settings,ssh,system,task,terminal,
|
||||
* workflow,…}`); each should be appended as it reaches zero. When the list would
|
||||
* cover all of `e2e/`, replace it with a plain `["e2e/**\/*.{ts,tsx}"]` — that is the
|
||||
* graduation, and at that point the diff-scanning ratchet becomes a second line
|
||||
* of defence rather than the only one.
|
||||
*/
|
||||
export const e2eSleepGuardFiles = [
|
||||
// Shard planning, flake and timing reports. Tooling rather than specs, and not
|
||||
// in any conversion chunk's path.
|
||||
"e2e/scripts/**/*.{ts,tsx}",
|
||||
// Test directories measured at zero raw sleeps.
|
||||
"e2e/tests/cli-mode/**/*.{ts,tsx}",
|
||||
"e2e/tests/docker/**/*.{ts,tsx}",
|
||||
"e2e/tests/github/**/*.{ts,tsx}",
|
||||
"e2e/tests/i18n/**/*.{ts,tsx}",
|
||||
"e2e/tests/kanban/**/*.{ts,tsx}",
|
||||
"e2e/tests/preview/**/*.{ts,tsx}",
|
||||
];
|
||||
|
||||
/**
|
||||
* The sanctioned waits. Calling one is only sanctioned if the identifier really
|
||||
* IS that helper — see {@link resolveWaitBinding}.
|
||||
*/
|
||||
const SANCTIONED_WAITS = new Set(["dwell", "injectLatency"]);
|
||||
|
||||
/** `…/causal-waits`, with or without an extension. */
|
||||
const CAUSAL_WAITS_MODULE = /(^|\/)causal-waits(\.[jt]s)?$/;
|
||||
|
||||
/** The nearest binding for `name`, walking out through enclosing scopes. */
|
||||
function lookupVariable(scope, name) {
|
||||
for (let current = scope; current; current = current.upper) {
|
||||
const found = current.variables.find((variable) => variable.name === name);
|
||||
if (found) return found;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `variable` is really the helper from `causal-waits`.
|
||||
*
|
||||
* An unresolved identifier is a `ReferenceError` waiting for the machine slow
|
||||
* enough to reach it, and a `dwell` imported from somewhere else is not the
|
||||
* helper this rule promotes. Both answer `false`.
|
||||
*/
|
||||
function isCausalWaitsBinding(variable) {
|
||||
if (!variable) return false;
|
||||
return variable.defs.some(
|
||||
(def) =>
|
||||
def.type === "ImportBinding" && CAUSAL_WAITS_MODULE.test(String(def.parent.source.value)),
|
||||
);
|
||||
}
|
||||
|
||||
const WAIT_FOR_TIMEOUT = "waitForTimeout";
|
||||
const SET_TIMEOUT = "setTimeout";
|
||||
|
||||
/** `setTimeout`, `window.setTimeout`, `globalThis.setTimeout`. */
|
||||
function isSetTimeoutCallee(node) {
|
||||
if (node.type === "Identifier") return node.name === SET_TIMEOUT;
|
||||
if (node.type !== "MemberExpression" || node.computed) return false;
|
||||
return node.property.type === "Identifier" && node.property.name === SET_TIMEOUT;
|
||||
}
|
||||
|
||||
/** The method name a call invokes, for `a.b()` and `a["b"]()` alike. */
|
||||
function calleeMethodName(node) {
|
||||
if (node.type === "Identifier") return node.name;
|
||||
if (node.type !== "MemberExpression") return null;
|
||||
if (!node.computed) return node.property.type === "Identifier" ? node.property.name : null;
|
||||
return node.property.type === "Literal" && typeof node.property.value === "string"
|
||||
? node.property.value
|
||||
: null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The executor's own statements, or `null` if it is not a function we can read.
|
||||
*
|
||||
* An expression-bodied arrow (`(r) => setTimeout(r, 5)`) is normalised to a
|
||||
* single-entry list so both forms flow through one check. Nested functions are
|
||||
* NOT unwrapped: that is exactly what keeps the rAF frame-yield out.
|
||||
*/
|
||||
function executorStatements(executor) {
|
||||
if (executor?.type !== "ArrowFunctionExpression" && executor?.type !== "FunctionExpression") {
|
||||
return null;
|
||||
}
|
||||
if (executor.body.type !== "BlockStatement") return [executor.body];
|
||||
return executor.body.body.map((statement) =>
|
||||
statement.type === "ExpressionStatement" ? statement.expression : statement,
|
||||
);
|
||||
}
|
||||
|
||||
/** The single expression a function evaluates to, or `null` if it does more. */
|
||||
function soleBodyExpression(node) {
|
||||
if (node.body.type !== "BlockStatement") return node.body;
|
||||
if (node.body.body.length !== 1) return null;
|
||||
const [only] = node.body.body;
|
||||
return only.type === "ExpressionStatement" ? only.expression : null;
|
||||
}
|
||||
|
||||
/** Whether `node` is `X.bind(...)`, returning `X`; otherwise `null`. */
|
||||
function boundTarget(node) {
|
||||
if (node.type !== "CallExpression") return null;
|
||||
const { callee } = node;
|
||||
if (callee.type !== "MemberExpression" || callee.computed) return null;
|
||||
if (callee.property.type !== "Identifier" || callee.property.name !== "bind") return null;
|
||||
return callee.object;
|
||||
}
|
||||
|
||||
/** Whether `node` is a call to `resolve`, or a function whose body only calls it. */
|
||||
function resolvesPromise(node, resolveName) {
|
||||
if (!node) return false;
|
||||
if (node.type === "Identifier") return node.name === resolveName;
|
||||
// A bound reference to `resolve` is still `resolve`:
|
||||
// `setTimeout(resolve.bind(null), ms)` sleeps exactly like `setTimeout(resolve, ms)`.
|
||||
// Recursive, so `resolve.bind(a).bind(b)` does not launder it either.
|
||||
const bound = boundTarget(node);
|
||||
if (bound) return resolvesPromise(bound, resolveName);
|
||||
if (node.type !== "ArrowFunctionExpression" && node.type !== "FunctionExpression") return false;
|
||||
|
||||
const body = soleBodyExpression(node);
|
||||
if (body?.type !== "CallExpression" || body.callee.type !== "Identifier") return false;
|
||||
return body.callee.name === resolveName;
|
||||
}
|
||||
|
||||
/**
|
||||
* The `setTimeout` call that makes `node` an unconditional sleep, or `null`.
|
||||
*
|
||||
* Every clause here is load-bearing — see the module comment for the real call
|
||||
* site each one protects.
|
||||
*/
|
||||
function promiseSleepCall(node) {
|
||||
if (node.callee.type !== "Identifier" || node.callee.name !== "Promise") return null;
|
||||
|
||||
const executor = node.arguments[0];
|
||||
const resolveParam = executor?.params?.[0];
|
||||
if (resolveParam?.type !== "Identifier") return null;
|
||||
|
||||
const statements = executorStatements(executor);
|
||||
// Exactly one statement: more than one means the timer is a fallback among
|
||||
// several resolution paths (a race guard), not the wait itself.
|
||||
if (statements?.length !== 1) return null;
|
||||
|
||||
const [only] = statements;
|
||||
// A bare call, so the handle is discarded. `timer = setTimeout(...)` arrives
|
||||
// as an AssignmentExpression and stops here: keeping the handle is the intent
|
||||
// to cancel, and a sleep is never cancelled.
|
||||
if (only.type !== "CallExpression" || !isSetTimeoutCallee(only.callee)) return null;
|
||||
|
||||
return resolvesPromise(only.arguments[0], resolveParam.name) ? only : null;
|
||||
}
|
||||
|
||||
/** @type {import("eslint").Rule.RuleModule} */
|
||||
export const noUnsanctionedSleep = {
|
||||
meta: {
|
||||
type: "problem",
|
||||
docs: {
|
||||
description:
|
||||
"Require dwell()/injectLatency() from e2e/helpers/causal-waits.ts instead of raw " +
|
||||
"page.waitForTimeout() or hand-rolled promise sleeps.",
|
||||
},
|
||||
schema: [],
|
||||
messages: {
|
||||
waitForTimeout:
|
||||
"Raw `waitForTimeout()` is not a sanctioned wait. Use `dwell(page, ms, category, reason)` " +
|
||||
"from `e2e/helpers/causal-waits.ts`, or better, wait on the cause with `waitForHttp` / " +
|
||||
"`watchWs`. See e2e/README.md.",
|
||||
unboundWait:
|
||||
"`{{name}}()` is not imported from `e2e/helpers/causal-waits`, so this is an " +
|
||||
"undefined identifier that throws at runtime — on exactly the slow, loaded shards a " +
|
||||
"wait is there to survive. Import it, or use a causal wait instead.",
|
||||
promiseSleep:
|
||||
"Hand-rolled promise sleep is not a sanctioned wait. Use `dwell(ms, category, reason)` " +
|
||||
"from `e2e/helpers/causal-waits.ts`, or `injectLatency(ms, reason)` inside a " +
|
||||
"`page.route()` handler. See e2e/README.md.",
|
||||
},
|
||||
},
|
||||
create(context) {
|
||||
return {
|
||||
CallExpression(node) {
|
||||
// A sanctioned wait is only sanctioned if the identifier resolves to the
|
||||
// real helper. `eslint.config.mjs` turns `no-undef` OFF repo-wide, and
|
||||
// nothing typechecks `e2e/`, so an import that silently failed to apply
|
||||
// leaves `dwell(...)` linting clean and throwing at runtime — inside
|
||||
// whichever retry path only executes under load. Scope analysis catches
|
||||
// this without any type information, which is how `no-undef` works.
|
||||
if (node.callee.type === "Identifier" && SANCTIONED_WAITS.has(node.callee.name)) {
|
||||
const variable = lookupVariable(context.sourceCode.getScope(node), node.callee.name);
|
||||
if (!isCausalWaitsBinding(variable)) {
|
||||
context.report({
|
||||
node: node.callee,
|
||||
messageId: "unboundWait",
|
||||
data: { name: node.callee.name },
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (calleeMethodName(node.callee) !== WAIT_FOR_TIMEOUT) return;
|
||||
// Report at the METHOD token, not the whole call. A finding's line is
|
||||
// its node's start line, and the ratchet keeps only findings on lines
|
||||
// the change added. For a wrapped call —
|
||||
//
|
||||
// page
|
||||
// .waitForTimeout(500);
|
||||
//
|
||||
// — the CallExpression starts at `page`, so changing `.click(…)` to
|
||||
// `.waitForTimeout(…)` would attribute the finding to an UNCHANGED line
|
||||
// and the ratchet would silently drop it. The method token is on the
|
||||
// line that actually changed.
|
||||
const target = node.callee.type === "MemberExpression" ? node.callee.property : node.callee;
|
||||
context.report({ node: target, messageId: "waitForTimeout" });
|
||||
},
|
||||
NewExpression(node) {
|
||||
const sleep = promiseSleepCall(node);
|
||||
if (sleep) context.report({ node: sleep, messageId: "promiseSleep" });
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
/** Flat-config plugin object, so both configs register the rule the same way. */
|
||||
export const e2eSleepPlugin = {
|
||||
rules: { "no-unsanctioned-sleep": noUnsanctionedSleep },
|
||||
};
|
||||
|
||||
export const SLEEP_RULE_ID = "e2e-sleeps/no-unsanctioned-sleep";
|
||||
@@ -0,0 +1,262 @@
|
||||
import { RuleTester } from "eslint";
|
||||
import tseslint from "typescript-eslint";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { noUnsanctionedSleep, SLEEP_EXEMPT_FILES } from "./no-unsanctioned-sleep.mjs";
|
||||
|
||||
/**
|
||||
* Every `invalid` case below is a shape that must FAIL, and every `valid` case
|
||||
* is a shape from this suite that must PASS. The valid list is the load-bearing
|
||||
* half: the rule's whole design problem is that `e2e/` holds 701
|
||||
* `test.setTimeout()` calls and a handful of race guards that look almost
|
||||
* exactly like the ~38 sleeps it must catch.
|
||||
*
|
||||
* Each valid case names the real call site it stands in for, so a future
|
||||
* loosening of the matcher fails here with the reason attached rather than
|
||||
* silently flagging correct code.
|
||||
*/
|
||||
/**
|
||||
* Hand vitest's `describe`/`it` to RuleTester explicitly.
|
||||
*
|
||||
* Without this, RuleTester finds no test globals (this project does not enable
|
||||
* `globals: true`) and falls back to running every case *synchronously during
|
||||
* collection*. The cases do execute, but vitest reports `Tests 1 passed (1)` —
|
||||
* only the explicit `it` below — so "25 cases passed" and "0 cases ran" produce
|
||||
* an identical, green-looking summary. Wiring the hooks makes each case a real
|
||||
* test, which is what lets `EXPECTED_CASES` below be an assertion rather than a
|
||||
* comment.
|
||||
*/
|
||||
RuleTester.describe = describe;
|
||||
RuleTester.it = it;
|
||||
|
||||
const ruleTester = new RuleTester({
|
||||
languageOptions: {
|
||||
parser: tseslint.parser as never,
|
||||
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
|
||||
},
|
||||
});
|
||||
|
||||
ruleTester.run("no-unsanctioned-sleep", noUnsanctionedSleep, {
|
||||
valid: [
|
||||
// --- The 701-site case. A regex ban on `setTimeout` dies here. ---
|
||||
{ code: "test.setTimeout(60_000);" },
|
||||
{ code: "e2eTest.setTimeout(120_000);" },
|
||||
|
||||
// --- The sanctioned forms, which are only sanctioned when actually imported. ---
|
||||
{
|
||||
code: `import { dwell } from "../../helpers/causal-waits";
|
||||
await dwell(page, 300, "library-timer", "Radix open delay publishes nothing");`,
|
||||
},
|
||||
{
|
||||
code: `import { dwell } from "./causal-waits";
|
||||
await dwell(500, "poll-interval", "backend health poll; no page exists yet");`,
|
||||
},
|
||||
{
|
||||
code: `import { injectLatency } from "../helpers/causal-waits";
|
||||
await injectLatency(800, "make the in-flight spinner observable");`,
|
||||
},
|
||||
// Aliased and namespace imports still resolve to the helper.
|
||||
{
|
||||
code: `import { dwell as sleep, injectLatency } from "../../helpers/causal-waits";
|
||||
await injectLatency(10, "x");`,
|
||||
},
|
||||
|
||||
// --- Race guard: several resolution paths, timer is a fallback. ---
|
||||
// e2e/tests/auth/auth-lifecycle.spec.ts:218
|
||||
{
|
||||
code: `new Promise<boolean>((resolve) => {
|
||||
const socket = new WebSocket(url);
|
||||
socket.onopen = () => resolve(false);
|
||||
socket.onclose = () => resolve(true);
|
||||
setTimeout(() => resolve(false), 3000);
|
||||
});`,
|
||||
},
|
||||
|
||||
// --- Cancellable timer: the handle is kept, so something clears it. ---
|
||||
// e2e/tests/ssh/concurrency.spec.ts:15 — one statement, still not a sleep.
|
||||
{
|
||||
code: `new Promise<{ kind: "timeout" }>((resolve) => {
|
||||
timer = setTimeout(() => resolve({ kind: "timeout" }), timeoutMs);
|
||||
});`,
|
||||
},
|
||||
{
|
||||
code: `new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => reject(new Error("timeout")), 30_000);
|
||||
ws.onmessage = () => { clearTimeout(timer); resolve(); };
|
||||
});`,
|
||||
},
|
||||
|
||||
// --- Frame yield, not a duration wait. No carve-out: the executor's one
|
||||
// --- statement is a requestAnimationFrame call, not a setTimeout one.
|
||||
// e2e/tests/layout/toggle-sidebar-shortcut.spec.ts:123
|
||||
{
|
||||
code: `new Promise<void>((resolve) => {
|
||||
requestAnimationFrame(() => setTimeout(resolve, 0));
|
||||
});`,
|
||||
},
|
||||
|
||||
// --- In-page code: the discriminator is what the promise RESOLVES ON,
|
||||
// --- not which context it runs in. A frame is an event; a duration is not.
|
||||
//
|
||||
// Instrumentation, sampling geometry frame by frame. The promise resolves on
|
||||
// the FRAME — the `setTimeout(…, 0)` only hops out of the rAF callback so the
|
||||
// measurement reflects painted geometry. Neither `dwell` nor `injectLatency`
|
||||
// exists in the browser context, so flagging this would trip the gate with no
|
||||
// legal way to comply. Excluded structurally (the executor's one statement is
|
||||
// the rAF call), and pinned here so a future tightening of the nesting rule
|
||||
// cannot silently break it.
|
||||
// e2e/tests/layout/toggle-sidebar-shortcut.spec.ts:123
|
||||
{
|
||||
code: `await page.evaluate(() =>
|
||||
new Promise<void>((resolve) => {
|
||||
requestAnimationFrame(() => setTimeout(resolve, 0));
|
||||
}),
|
||||
);`,
|
||||
},
|
||||
|
||||
// --- A promise that is not a sleep at all. ---
|
||||
{ code: "new Promise((resolve) => { socket.onopen = () => resolve(); });" },
|
||||
{ code: "new Promise((resolve) => setTimeout(unrelatedCallback, 100));" },
|
||||
// Not the global Promise constructor shape the rule keys on.
|
||||
{ code: "new Deferred((resolve) => setTimeout(resolve, 100));" },
|
||||
],
|
||||
|
||||
invalid: [
|
||||
// --- waitForTimeout, in every receiver position Playwright offers. ---
|
||||
{
|
||||
code: "await page.waitForTimeout(500);",
|
||||
errors: [{ messageId: "waitForTimeout" }],
|
||||
},
|
||||
{
|
||||
code: "await locator.page().waitForTimeout(1000);",
|
||||
errors: [{ messageId: "waitForTimeout" }],
|
||||
},
|
||||
{
|
||||
code: 'await page["waitForTimeout"](500);',
|
||||
errors: [{ messageId: "waitForTimeout" }],
|
||||
},
|
||||
{
|
||||
code: "await frame.waitForTimeout(HEALTH_POLL_MS);",
|
||||
errors: [{ messageId: "waitForTimeout" }],
|
||||
},
|
||||
|
||||
// --- Promise sleeps, every form the suite actually contains. ---
|
||||
// The 26-site shape.
|
||||
{
|
||||
code: "await new Promise((r) => setTimeout(r, 500));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// The 6-site shape.
|
||||
{
|
||||
code: "await new Promise((resolve) => setTimeout(resolve, 1000));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// The typed shape.
|
||||
{
|
||||
code: "await new Promise<void>((resolve) => setTimeout(resolve, 250));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// Block-bodied, still exactly one statement.
|
||||
{
|
||||
code: "await new Promise((resolve) => { setTimeout(resolve, 300); });",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// A wrapping arrow that only calls resolve is still a sleep.
|
||||
{
|
||||
code: "await new Promise((resolve) => setTimeout(() => resolve(), 300));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// Non-literal delays are sleeps too.
|
||||
{
|
||||
code: "await new Promise((r) => setTimeout(r, HEALTH_POLL_MS));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// `window.`/`globalThis.` prefix must not launder it.
|
||||
{
|
||||
code: "await new Promise((r) => window.setTimeout(r, 100));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// A function expression executor, not just an arrow.
|
||||
{
|
||||
code: "await new Promise(function (resolve) { setTimeout(resolve, 100); });",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// Both bans can fire in one file.
|
||||
{
|
||||
code: "await page.waitForTimeout(100); await new Promise((r) => setTimeout(r, 100));",
|
||||
errors: [{ messageId: "waitForTimeout" }, { messageId: "promiseSleep" }],
|
||||
},
|
||||
// A sanctioned wait that is not actually bound to the helper. Chunk 1 hit
|
||||
// this for real: a patch anchored on an import line whose actual form was
|
||||
// `{ type Page, expect }`, so the edit missed and the call site was left
|
||||
// referencing an undefined identifier. `no-undef` is off repo-wide and
|
||||
// nothing typechecks `e2e/`, so lint passed clean and it would have surfaced
|
||||
// as `dwell is not defined` inside a retry path that only runs on slow
|
||||
// shards. The token being allowed is not the same as the helper being there.
|
||||
{
|
||||
code: 'await dwell(page, 500, "unverified", "import silently did not apply");',
|
||||
errors: [{ messageId: "unboundWait" }],
|
||||
},
|
||||
{
|
||||
code: 'await injectLatency(200, "import silently did not apply");',
|
||||
errors: [{ messageId: "unboundWait" }],
|
||||
},
|
||||
// Imported, but not from causal-waits: also not the sanctioned helper.
|
||||
{
|
||||
code: `import { dwell } from "./local-utils";
|
||||
await dwell(500, "unverified", "wrong module");`,
|
||||
errors: [{ messageId: "unboundWait" }],
|
||||
},
|
||||
// A locally defined look-alike is not the helper either.
|
||||
{
|
||||
code: `function dwell(ms) { return ms; }
|
||||
await dwell(500, "unverified", "shadowed by a local");`,
|
||||
errors: [{ messageId: "unboundWait" }],
|
||||
},
|
||||
|
||||
// A DURATION wait is still a duration wait inside `page.evaluate`.
|
||||
// `await page.evaluate(() => new Promise((r) => setTimeout(r, 500)))` blocks
|
||||
// the test for 500ms exactly like `page.waitForTimeout(500)` — the browser
|
||||
// context is a wrapper, not a different kind of wait. Exempting in-page code
|
||||
// wholesale would make that one line a laundering wrapper for every sleep in
|
||||
// the suite. There IS a legal path: hoist it to `dwell(page, ms, …)` and
|
||||
// evaluate afterwards, or resolve on a frame/event as the rAF case above does.
|
||||
{
|
||||
code: "await page.evaluate(() => new Promise((r) => setTimeout(r, 500)));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// A bound reference to `resolve` is still `resolve`. Flagged so the form
|
||||
// cannot be used to launder a sleep past the rule (review finding).
|
||||
{
|
||||
code: "await new Promise((resolve) => setTimeout(resolve.bind(null), 100));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
{
|
||||
code: "await new Promise((resolve) => setTimeout(resolve.bind(null).bind(null), 100));",
|
||||
errors: [{ messageId: "promiseSleep" }],
|
||||
},
|
||||
// The finding must land on the METHOD token, not the receiver. On a
|
||||
// wrapped call those are different LINES, and the ratchet keeps only
|
||||
// findings on lines the change added — so reporting the whole call would
|
||||
// attribute this to line 1 and silently drop it (review finding).
|
||||
{
|
||||
code: "await page\n .waitForTimeout(500);",
|
||||
errors: [{ messageId: "waitForTimeout", line: 2 }],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
describe("no-unsanctioned-sleep exemptions", () => {
|
||||
/**
|
||||
* The exemption list is consumed by two separate eslint configs. Asserting the
|
||||
* paths exist stops a rename from silently un-exempting the helper that
|
||||
* implements both banned forms — which would surface as the sanctioned wait
|
||||
* failing the rule that exists to promote it.
|
||||
*/
|
||||
it("exempts exactly the causal-waits implementation and its own test", () => {
|
||||
expect(SLEEP_EXEMPT_FILES).toEqual([
|
||||
"e2e/helpers/causal-waits.ts",
|
||||
"e2e/helpers/causal-waits.test.ts",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,11 @@ import unusedImports from "eslint-plugin-unused-imports";
|
||||
import i18next from "eslint-plugin-i18next";
|
||||
import tseslint from "typescript-eslint";
|
||||
import { i18nGuardFiles, noLiteralStringOptions } from "./eslint.i18n.options.mjs";
|
||||
import {
|
||||
e2eSleepGuardFiles,
|
||||
e2eSleepPlugin,
|
||||
SLEEP_EXEMPT_FILES,
|
||||
} from "./eslint-rules/no-unsanctioned-sleep.mjs";
|
||||
|
||||
const eslintConfig = defineConfig([
|
||||
{
|
||||
@@ -77,6 +82,21 @@ const eslintConfig = defineConfig([
|
||||
"sonarjs/no-duplicate-string": "off",
|
||||
},
|
||||
},
|
||||
// Unconditional wall-clock sleeps in E2E specs. An ERROR, but only on
|
||||
// `e2eSleepGuardFiles` — the directories the causal-waits conversion has
|
||||
// already driven to zero. `pnpm lint` is `eslint --max-warnings 0`, so a
|
||||
// repo-wide registration at ANY severity (warning included) would fail the
|
||||
// build on the ~126 sleeps that predate the rule. Everything not listed is
|
||||
// covered by `scripts/check-new-e2e-sleeps.mjs`, which judges only the lines a
|
||||
// change added, so new sleeps are caught everywhere without a treadmill.
|
||||
// Append a directory here when it reaches zero; see the list's own comment.
|
||||
{
|
||||
files: e2eSleepGuardFiles,
|
||||
// `causal-waits.ts` implements both banned forms — see SLEEP_EXEMPT_FILES.
|
||||
ignores: SLEEP_EXEMPT_FILES,
|
||||
plugins: { "e2e-sleeps": e2eSleepPlugin },
|
||||
rules: { "e2e-sleeps/no-unsanctioned-sleep": "error" },
|
||||
},
|
||||
]);
|
||||
|
||||
export default eslintConfig;
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { defineConfig } from "eslint/config";
|
||||
import tseslint from "typescript-eslint";
|
||||
|
||||
import { e2eSleepPlugin, SLEEP_EXEMPT_FILES } from "./eslint-rules/no-unsanctioned-sleep.mjs";
|
||||
|
||||
/**
|
||||
* Standalone config for the sleep ratchet and its preview command.
|
||||
*
|
||||
* The main config (eslint.config.mjs) errors only on `e2eSleepGuardFiles` — the
|
||||
* directories the causal-waits conversion has already driven to zero. That is
|
||||
* deliberate: `pnpm run lint` is `eslint --max-warnings 0`, so registering this
|
||||
* rule repo-wide at *any* severity, warning included, would fail the build on
|
||||
* the ~130 sleeps that predate it and break every unrelated PR until the
|
||||
* conversion finishes.
|
||||
*
|
||||
* This config applies the same rule to whatever paths it is given, regardless of
|
||||
* that allowlist. Two consumers:
|
||||
*
|
||||
* - `scripts/check-new-e2e-sleeps.mjs`, the ratchet, which filters findings
|
||||
* down to the lines a change added.
|
||||
* - `pnpm run lint:e2e-sleeps <path>`, the work-list generator for whoever is
|
||||
* converting a directory next.
|
||||
*
|
||||
* The rule and the exemption list are imported, not restated, so the preview,
|
||||
* the gate and the editor cannot disagree about what counts as a sleep.
|
||||
*/
|
||||
export default defineConfig([
|
||||
{
|
||||
// BOTH extensions. `isE2eCandidate` selects `.tsx` too, so a config matching
|
||||
// only `.ts` left a selected file matching no config at all — and
|
||||
// `warnIgnored: false` in the ratchet suppressed the one signal that would
|
||||
// have shown it, so the gate passed silently. Keep this in step with the
|
||||
// selector.
|
||||
files: ["**/*.ts", "**/*.tsx"],
|
||||
ignores: [...SLEEP_EXEMPT_FILES, "node_modules/**", "dist/**"],
|
||||
languageOptions: { parser: tseslint.parser },
|
||||
plugins: { "e2e-sleeps": e2eSleepPlugin },
|
||||
rules: { "e2e-sleeps/no-unsanctioned-sleep": "error" },
|
||||
},
|
||||
]);
|
||||
@@ -12,6 +12,7 @@
|
||||
"start": "vite preview --host 0.0.0.0",
|
||||
"lint": "eslint --max-warnings 0",
|
||||
"lint:i18n": "eslint --config eslint.i18n.config.mjs",
|
||||
"lint:e2e-sleeps": "eslint --config eslint.e2e-sleeps.config.mjs",
|
||||
"pretypecheck": "node scripts/generate-release-notes.mjs && node scripts/generate-changelog.mjs",
|
||||
"typecheck": "tsc --noEmit",
|
||||
"pretest": "node scripts/generate-release-notes.mjs && node scripts/generate-changelog.mjs",
|
||||
@@ -23,6 +24,8 @@
|
||||
"e2e:ui": "playwright test --config e2e/playwright.config.ts --ui",
|
||||
"e2e:headed": "playwright test --config e2e/playwright.config.ts --headed",
|
||||
"e2e:capture": "cross-env CAPTURE_PR_ASSETS=true playwright test --config e2e/playwright.config.ts && tsx e2e/scripts/convert-assets.ts",
|
||||
"e2e:sleep-ratchet": "node scripts/check-new-e2e-sleeps.mjs",
|
||||
"e2e:waits": "node scripts/report-e2e-waits.mjs",
|
||||
"licenses:gen": "tsx scripts/generate-licenses.ts",
|
||||
"i18n:pseudo": "node scripts/generate-pseudo-locale.mjs",
|
||||
"i18n:check": "node scripts/check-i18n-keys.mjs && node scripts/check-trans-indices.mjs && node scripts/check-inline-plurals.mjs && node scripts/check-module-scope-t.mjs && node scripts/check-no-em-dash-ui.mjs",
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Ratchet: NEW unconditional sleeps in `apps/web/e2e` must go through `dwell()`
|
||||
* or `injectLatency()` from `e2e/helpers/causal-waits.ts`.
|
||||
*
|
||||
* `page.waitForTimeout(500)` and `await new Promise((r) => setTimeout(r, 500))`
|
||||
* are the suite's most reliable source of flake: a hand-picked number that
|
||||
* happened to be long enough on the machine it was written on. The sanctioned
|
||||
* forms take a category and a reason, so a wall-clock wait that survives review
|
||||
* has to say why no event exists to wait on instead.
|
||||
*
|
||||
* ~126 raw sleeps predate this check, and `pnpm lint` is
|
||||
* `eslint --max-warnings 0`, so registering the rule repo-wide at any severity
|
||||
* would break every unrelated PR until the conversion finishes — the mistake the
|
||||
* first i18n migration attempt made. Hence judging the CHANGE, not the FILE:
|
||||
*
|
||||
* - a file the change ADDED must be clean outright. New code carries no legacy
|
||||
* debt, so requiring zero sleeps costs nothing.
|
||||
* - a file the change MODIFIED is judged only on the lines it touched. Sleeps
|
||||
* elsewhere in the file belong to the conversion effort, not to this PR.
|
||||
*
|
||||
* Same contract, same plumbing and same fork-point handling as
|
||||
* `check-new-code-i18n.mjs`, which mirrors `golangci-lint --new-from-rev`.
|
||||
*
|
||||
* The decision logic lives in `lib/e2e-sleep-ratchet.mjs` so it can be driven
|
||||
* against real git topologies in a test; this file is the shell that supplies
|
||||
* the real repository and turns the result into output and an exit code.
|
||||
*
|
||||
* Usage:
|
||||
* node scripts/check-new-e2e-sleeps.mjs [--base <ref-or-sha>]
|
||||
*
|
||||
* Prefer passing no --base; see lib/git-base.mjs for why an explicit one is
|
||||
* floored at the fork point.
|
||||
*/
|
||||
import path from "node:path";
|
||||
|
||||
import { findNewSleeps } from "./lib/e2e-sleep-ratchet.mjs";
|
||||
import { REPO_ROOT, WEB_DIR } from "./lib/git-base.mjs";
|
||||
|
||||
const result = await findNewSleeps({
|
||||
repoRoot: REPO_ROOT,
|
||||
webDir: WEB_DIR,
|
||||
configFile: path.join(WEB_DIR, "eslint.e2e-sleeps.config.mjs"),
|
||||
argv: process.argv,
|
||||
});
|
||||
|
||||
if (result.skip) {
|
||||
console.log(`⚠ e2e sleep ratchet skipped — ${result.skip}`);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const { added, modified, violations } = result;
|
||||
|
||||
if (added.length === 0 && modified.length === 0) {
|
||||
console.log("✓ e2e sleep ratchet — no e2e source added or modified");
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
if (violations.length === 0) {
|
||||
console.log(
|
||||
`✓ e2e sleep ratchet — ${added.length} added + ${modified.length} modified e2e file(s) clean`,
|
||||
);
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
console.error(
|
||||
`\n✖ ${violations.length} unconditional sleep(s) in new e2e code.\n` +
|
||||
` A wall-clock wait must name its category and its reason, so use dwell() or\n` +
|
||||
` injectLatency() from e2e/helpers/causal-waits.ts. Better still, wait on the\n` +
|
||||
` cause: waitForHttp() / watchWs(). See apps/web/e2e/README.md.\n`,
|
||||
);
|
||||
for (const violation of violations) {
|
||||
const scope = violation.whole ? "new file" : "changed line";
|
||||
console.error(` ${violation.repoPath}:${violation.line} [${scope}] ${violation.text}`);
|
||||
}
|
||||
console.error("");
|
||||
process.exit(1);
|
||||
@@ -21,18 +21,56 @@ export function isCandidate(repoPath) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Candidate paths matching `filter` (a `--diff-filter` value) between `base` and
|
||||
* the working tree.
|
||||
* Only Playwright E2E sources, for the sleep ratchet.
|
||||
*
|
||||
* The exact complement of the `e2e/` exclusion in {@link isCandidate}: the i18n
|
||||
* guard skips this tree because tests build fixtures out of literals on purpose,
|
||||
* and the sleep guard exists only for this tree. `.spec.ts` files are the
|
||||
* *primary* target here rather than an exclusion, which is why this cannot be
|
||||
* expressed as options on the other predicate.
|
||||
*
|
||||
* `causal-waits.ts` is not excluded here. Exemption belongs to the eslint config
|
||||
* (`SLEEP_EXEMPT_FILES`), so the editor, the preview and the gate all read one
|
||||
* list; filtering it out at selection time as well would be a second place to
|
||||
* keep in sync, and the one that fails silently.
|
||||
*/
|
||||
export function isE2eCandidate(repoPath) {
|
||||
if (!repoPath.startsWith(WEB_PREFIX)) return false;
|
||||
const rel = repoPath.slice(WEB_PREFIX.length);
|
||||
return rel.startsWith("e2e/") && /\.tsx?$/.test(rel);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every path matching `filter` (a `--diff-filter` value) between `base` and the
|
||||
* working tree, unfiltered.
|
||||
*
|
||||
* `--find-renames` explicitly: rename detection defaults on since Git 2.9, but a
|
||||
* repo with diff.renames=false would report a moved file as ADDED, and an added
|
||||
* file is judged whole — demanding a full migration for a plain `git mv`.
|
||||
*/
|
||||
export function changedPaths(base, filter, cwd) {
|
||||
return (
|
||||
git(["diff", "--name-only", "--find-renames", `--diff-filter=${filter}`, base], { cwd })
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
// `"".split("\n")` is `[""]`, so a no-match diff would otherwise yield one
|
||||
// empty "path". Both predicates reject it, but this function is exported and
|
||||
// its next caller should not have to know that.
|
||||
.filter(Boolean)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Candidate UI-source paths matching `filter`.
|
||||
*
|
||||
* A pure delegation to {@link changedPaths} + {@link isCandidate}: the i18n
|
||||
* ratchet's file selection is byte-for-byte what it was before the sleep ratchet
|
||||
* needed the same git invocation with a different predicate. Kept as its own
|
||||
* export rather than inlined at the call site so that stays true by
|
||||
* construction.
|
||||
*/
|
||||
export function changedFiles(base, filter, cwd) {
|
||||
return git(["diff", "--name-only", "--find-renames", `--diff-filter=${filter}`, base], { cwd })
|
||||
.split("\n")
|
||||
.map((line) => line.trim())
|
||||
.filter(isCandidate);
|
||||
return changedPaths(base, filter, cwd).filter(isCandidate);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { isCandidate, isE2eCandidate } from "./changed-files.mjs";
|
||||
|
||||
/**
|
||||
* `isCandidate` decides which files the **live i18n ratchet** judges, and had no
|
||||
* test until the sleep ratchet needed the same git plumbing with a different
|
||||
* predicate. These cases pin the existing behaviour so that extraction is
|
||||
* provably behaviour-preserving, and pin the new predicate as its exact
|
||||
* complement over the `e2e/` tree.
|
||||
*/
|
||||
describe("isCandidate (i18n ratchet file selection)", () => {
|
||||
it.each([
|
||||
"apps/web/components/task/card.tsx",
|
||||
"apps/web/app/settings/page.tsx",
|
||||
"apps/web/hooks/domains/kanban/use-board.ts",
|
||||
"apps/web/lib/state/store.ts",
|
||||
"apps/web/src/settings-routes.tsx",
|
||||
])("includes UI source %s", (path) => {
|
||||
expect(isCandidate(path)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
// Tests build fixtures out of literals on purpose.
|
||||
["apps/web/components/task/card.test.tsx", "unit test"],
|
||||
["apps/web/lib/util.spec.ts", "spec file"],
|
||||
// Not UI source.
|
||||
["apps/web/e2e/tests/task/chat.spec.ts", "e2e tree"],
|
||||
["apps/web/scripts/check-i18n-keys.mjs", "build script"],
|
||||
["apps/web/eslint.config.mjs", "config, not .ts"],
|
||||
["apps/web/components/task/styles.css", "not TypeScript"],
|
||||
["apps/backend/internal/office/router.go", "outside apps/web"],
|
||||
["apps/packages/ui/src/button.tsx", "outside apps/web"],
|
||||
])("excludes %s (%s)", (path) => {
|
||||
expect(isCandidate(path)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isE2eCandidate (sleep ratchet file selection)", () => {
|
||||
it.each([
|
||||
"apps/web/e2e/tests/task/chat.spec.ts",
|
||||
"apps/web/e2e/helpers/api-client.ts",
|
||||
"apps/web/e2e/fixtures/backend.ts",
|
||||
"apps/web/e2e/pages/task-page.ts",
|
||||
// Exemption is the eslint config's job, not the selector's — this path is
|
||||
// deliberately still selected here.
|
||||
"apps/web/e2e/helpers/causal-waits.ts",
|
||||
])("includes %s", (path) => {
|
||||
expect(isE2eCandidate(path)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["apps/web/components/task/card.tsx", "UI source, not e2e"],
|
||||
["apps/web/scripts/check-new-e2e-sleeps.mjs", "build script"],
|
||||
["apps/web/e2e/README.md", "not TypeScript"],
|
||||
["apps/web/e2e/scripts/seed.js", "plain JS is not parsed by the rule config"],
|
||||
["apps/backend/internal/office/router.go", "outside apps/web"],
|
||||
])("excludes %s (%s)", (path) => {
|
||||
expect(isE2eCandidate(path)).toBe(false);
|
||||
});
|
||||
|
||||
/**
|
||||
* The two predicates must not both claim a file: the i18n guard would demand
|
||||
* catalog keys for test fixture strings, which is why `e2e/` was excluded from
|
||||
* it in the first place.
|
||||
*/
|
||||
it("never overlaps with the i18n predicate", () => {
|
||||
const paths = [
|
||||
"apps/web/e2e/tests/task/chat.spec.ts",
|
||||
"apps/web/e2e/helpers/api-client.ts",
|
||||
"apps/web/components/task/card.tsx",
|
||||
"apps/web/lib/state/store.ts",
|
||||
];
|
||||
for (const path of paths) {
|
||||
expect(isCandidate(path) && isE2eCandidate(path)).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
/**
|
||||
* The sleep ratchet's decision logic, separated from its CLI shell.
|
||||
*
|
||||
* Split out so the ratchet can be driven against real git topologies in a test
|
||||
* — a temp repository with the same `apps/web/e2e/…` layout — rather than only
|
||||
* through its individual helpers. Testing `parseAddedLineRanges` and the eslint
|
||||
* rule proves each part works; it does not prove the parts are wired together,
|
||||
* and a gate that is perfectly tested but never actually consulted reports a
|
||||
* clean tree forever. `findNewSleeps` is the seam where those two claims meet,
|
||||
* so it is the thing the tests drive.
|
||||
*
|
||||
* Everything path-shaped is a parameter for the same reason. The CLI supplies
|
||||
* the real repository; the tests supply a throwaway one.
|
||||
*/
|
||||
import path from "node:path";
|
||||
|
||||
import { ESLint } from "eslint";
|
||||
|
||||
import { SLEEP_RULE_ID } from "../../eslint-rules/no-unsanctioned-sleep.mjs";
|
||||
import { changedPaths, isE2eCandidate, webDiff } from "./changed-files.mjs";
|
||||
import { lineInRanges, parseAddedLineRanges } from "./diff-ranges.mjs";
|
||||
import { resolveBase, toPosixPath } from "./git-base.mjs";
|
||||
|
||||
/**
|
||||
* Sleeps introduced by the change between the resolved base and the working
|
||||
* tree.
|
||||
*
|
||||
* Added files are judged whole; modified files only on the lines they touched.
|
||||
*
|
||||
* @returns `{ skip }` when no base could be resolved and skipping is allowed,
|
||||
* otherwise `{ base, added, modified, violations }`.
|
||||
*/
|
||||
export async function findNewSleeps({ repoRoot, webDir, configFile, argv = [] }) {
|
||||
const resolved = resolveBase(argv, repoRoot);
|
||||
if (resolved.skip) return { skip: resolved.skip };
|
||||
const { base } = resolved;
|
||||
|
||||
const added = changedPaths(base, "A", repoRoot).filter(isE2eCandidate);
|
||||
// Renames are judged like modifications: moving a legacy spec must not
|
||||
// suddenly demand that every sleep in it be converted. A pure rename adds no
|
||||
// lines and so reports nothing.
|
||||
const modified = changedPaths(base, "MRC", repoRoot).filter(isE2eCandidate);
|
||||
const targets = [...added, ...modified];
|
||||
if (targets.length === 0) return { base, added, modified, violations: [] };
|
||||
|
||||
const addedRanges =
|
||||
modified.length === 0 ? new Map() : parseAddedLineRanges(webDiff(base, repoRoot));
|
||||
|
||||
const eslint = new ESLint({
|
||||
cwd: webDir,
|
||||
overrideConfigFile: configFile,
|
||||
// `causal-waits.ts` is in the config's `ignores`, and passing an ignored
|
||||
// file explicitly is a warning that would otherwise print on every run that
|
||||
// touches it.
|
||||
warnIgnored: false,
|
||||
});
|
||||
const results = await eslint.lintFiles(targets.map((file) => path.join(repoRoot, file)));
|
||||
|
||||
const addedSet = new Set(added);
|
||||
const violations = [];
|
||||
for (const result of results) {
|
||||
const repoPath = toPosixPath(path.relative(repoRoot, result.filePath));
|
||||
const whole = addedSet.has(repoPath);
|
||||
const ranges = addedRanges.get(repoPath);
|
||||
for (const message of result.messages) {
|
||||
if (message.ruleId !== SLEEP_RULE_ID) continue;
|
||||
if (!whole && !lineInRanges(ranges, message.line)) continue;
|
||||
violations.push({
|
||||
repoPath,
|
||||
whole,
|
||||
line: message.line,
|
||||
messageId: message.messageId,
|
||||
text: message.message,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return { base, added, modified, violations };
|
||||
}
|
||||
@@ -0,0 +1,312 @@
|
||||
/**
|
||||
* The sleep ratchet driven end to end against real git topologies.
|
||||
*
|
||||
* The rule has its own RuleTester suite and the diff parser has its own; those
|
||||
* prove the parts. This proves they are *connected* — that a sleep on a new line
|
||||
* actually reaches an exit code, and that a sleep on an untouched line actually
|
||||
* does not. A gate whose matcher is perfect but whose wiring is broken reports a
|
||||
* clean tree forever, which is the one failure mode nobody goes looking for.
|
||||
*
|
||||
* Every scenario is a real repository with the real `apps/web/e2e/…` layout and
|
||||
* the real eslint config, so nothing here can pass by mocking the thing under
|
||||
* test.
|
||||
*/
|
||||
import { execFileSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, describe, expect, it } from "vitest";
|
||||
|
||||
import { findNewSleeps } from "./e2e-sleep-ratchet.mjs";
|
||||
|
||||
const REAL_WEB_DIR = path.resolve(import.meta.dirname, "../..");
|
||||
const CONFIG_FILE = path.join(REAL_WEB_DIR, "eslint.e2e-sleeps.config.mjs");
|
||||
|
||||
const SPEC = "apps/web/e2e/tests/demo/demo.spec.ts";
|
||||
|
||||
/** The line each scenario inserts before, extracted because it appears in five. */
|
||||
const ANCHOR = ' await page.click("#done");';
|
||||
|
||||
/** A spec with a sleep at a known line, plus filler to edit elsewhere. */
|
||||
const LEGACY_SPEC = [
|
||||
'import { test } from "@playwright/test";', // 1
|
||||
"", // 2
|
||||
'test("legacy", async ({ page }) => {', // 3
|
||||
' await page.goto("/");', // 4
|
||||
" await page.waitForTimeout(500);", // 5 <- the pre-existing sleep
|
||||
ANCHOR, // 6
|
||||
"});", // 7
|
||||
"", // 8
|
||||
].join("\n");
|
||||
|
||||
const tmpRoots: string[] = [];
|
||||
afterAll(() => {
|
||||
for (const dir of tmpRoots) fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function git(cwd: string, args: string[]) {
|
||||
return execFileSync(
|
||||
"git",
|
||||
[
|
||||
"-c",
|
||||
"user.email=ratchet@example.test",
|
||||
"-c",
|
||||
"user.name=Ratchet Test",
|
||||
"-c",
|
||||
"commit.gpgsign=false",
|
||||
// A developer machine may set core.hooksPath globally; an inherited
|
||||
// pre-commit hook would fail these fixture commits for unrelated reasons.
|
||||
"-c",
|
||||
"core.hooksPath=/nonexistent",
|
||||
...args,
|
||||
],
|
||||
{ cwd, encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] },
|
||||
);
|
||||
}
|
||||
|
||||
function write(dir: string, rel: string, body: string) {
|
||||
const file = path.join(dir, rel);
|
||||
fs.mkdirSync(path.dirname(file), { recursive: true });
|
||||
fs.writeFileSync(file, body);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stage everything, which is what makes a NEW file visible to the ratchet.
|
||||
*
|
||||
* `git diff <base>` compares a commit against the working tree and does not
|
||||
* report untracked paths at all, so an unstaged new file is invisible here. That
|
||||
* is not a gap: pre-commit runs its hooks after staging, and CI judges commits,
|
||||
* so every path that reaches this gate for real is tracked. Modified files need
|
||||
* no staging, and are deliberately left unstaged below to prove the ratchet does
|
||||
* not depend on it.
|
||||
*/
|
||||
function stage(dir: string) {
|
||||
git(dir, ["add", "-A"]);
|
||||
}
|
||||
|
||||
/**
|
||||
* A repo whose `main` already contains `LEGACY_SPEC`, with `origin/main`
|
||||
* pointing at it — the shape `resolveBase` expects — and HEAD on a branch off
|
||||
* that commit, so working-tree edits read as this change's own.
|
||||
*/
|
||||
function repoWithLegacySleep() {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-sleep-ratchet-"));
|
||||
tmpRoots.push(dir);
|
||||
git(dir, ["init", "-q", "-b", "main"]);
|
||||
write(dir, SPEC, LEGACY_SPEC);
|
||||
git(dir, ["add", "-A"]);
|
||||
git(dir, ["commit", "-q", "-m", "base"]);
|
||||
// `origin/main` without a remote: a local ref under refs/remotes is all
|
||||
// `merge-base origin/main HEAD` needs.
|
||||
git(dir, ["update-ref", "refs/remotes/origin/main", "HEAD"]);
|
||||
git(dir, ["checkout", "-q", "-b", "feature"]);
|
||||
return dir;
|
||||
}
|
||||
|
||||
const run = (dir: string) =>
|
||||
findNewSleeps({
|
||||
repoRoot: dir,
|
||||
webDir: path.join(dir, "apps/web"),
|
||||
configFile: CONFIG_FILE,
|
||||
}) as Promise<{
|
||||
violations: Array<{ repoPath: string; line: number; whole: boolean; messageId: string }>;
|
||||
added: string[];
|
||||
modified: string[];
|
||||
}>;
|
||||
|
||||
describe("e2e sleep ratchet", () => {
|
||||
it("FAILS on a waitForTimeout added on a new line", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(
|
||||
dir,
|
||||
SPEC,
|
||||
LEGACY_SPEC.replace(
|
||||
ANCHOR,
|
||||
' await page.waitForTimeout(250);\n await page.click("#done");',
|
||||
),
|
||||
);
|
||||
|
||||
const { violations } = await run(dir);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({ line: 6, whole: false, messageId: "waitForTimeout" });
|
||||
});
|
||||
|
||||
it("FAILS on a raw promise sleep added on a new line", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(
|
||||
dir,
|
||||
SPEC,
|
||||
LEGACY_SPEC.replace(
|
||||
ANCHOR,
|
||||
' await new Promise((r) => setTimeout(r, 250));\n await page.click("#done");',
|
||||
),
|
||||
);
|
||||
|
||||
const { violations } = await run(dir);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({ line: 6, whole: false, messageId: "promiseSleep" });
|
||||
});
|
||||
|
||||
/**
|
||||
* The property that makes this mergeable at all: touching a line next to
|
||||
* somebody else's sleep must not make it yours. Without it the ratchet is a
|
||||
* migration treadmill and the first PR to edit a legacy spec is blocked.
|
||||
*/
|
||||
it("PASSES when a line is edited right next to an existing untouched sleep", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(
|
||||
dir,
|
||||
SPEC,
|
||||
LEGACY_SPEC.replace(' await page.goto("/");', ' await page.goto("/tasks");'),
|
||||
);
|
||||
|
||||
const { modified, violations } = await run(dir);
|
||||
expect(modified).toEqual([SPEC]);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
it("PASSES on an existing untouched sleep with no changes at all", async () => {
|
||||
const { modified, added, violations } = await run(repoWithLegacySleep());
|
||||
expect(added).toEqual([]);
|
||||
expect(modified).toEqual([]);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
it("PASSES on the sanctioned dwell() and injectLatency() forms", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(
|
||||
dir,
|
||||
SPEC,
|
||||
LEGACY_SPEC.replace(
|
||||
'import { test } from "@playwright/test";',
|
||||
'import { test } from "@playwright/test";\nimport { dwell, injectLatency } from "../../helpers/causal-waits";',
|
||||
).replace(
|
||||
ANCHOR,
|
||||
[
|
||||
' await dwell(page, 300, "library-timer", "Radix open delay publishes nothing");',
|
||||
' await dwell(300, "poll-interval", "backend health poll; no page here");',
|
||||
' await injectLatency(800, "make the in-flight spinner observable");',
|
||||
ANCHOR,
|
||||
].join("\n"),
|
||||
),
|
||||
);
|
||||
|
||||
const { violations } = await run(dir);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* File-level selection: which paths the gate judges whole, which it judges by
|
||||
* line, and which it does not judge at all.
|
||||
*/
|
||||
describe("e2e sleep ratchet — file selection", () => {
|
||||
/** An added file carries no legacy debt, so it is judged whole. */
|
||||
it("FAILS on a sleep anywhere in a newly ADDED file", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(dir, "apps/web/e2e/tests/demo/new.spec.ts", LEGACY_SPEC);
|
||||
stage(dir);
|
||||
|
||||
const { added, violations } = await run(dir);
|
||||
expect(added).toEqual(["apps/web/e2e/tests/demo/new.spec.ts"]);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({ line: 5, whole: true });
|
||||
});
|
||||
|
||||
/**
|
||||
* A `git mv` of a legacy spec adds no lines, so it must report nothing.
|
||||
* Judging a rename as an addition would demand a full conversion for a move.
|
||||
*/
|
||||
it("PASSES on a pure rename of a file full of legacy sleeps", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
git(dir, ["mv", SPEC, "apps/web/e2e/tests/demo/renamed.spec.ts"]);
|
||||
|
||||
const { violations } = await run(dir);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* `.tsx` is selected by `isE2eCandidate`, so the eslint config must match it
|
||||
* too. It did not: the file was linted against no config at all, and
|
||||
* `warnIgnored: false` suppressed the only signal that would have shown it, so
|
||||
* the gate PASSED on a file full of sleeps. Found in review; this is the
|
||||
* regression test, and it fails against a `files: ["**\/*.ts"]` config.
|
||||
*/
|
||||
it("FAILS on a sleep in a .tsx file, which the selector also picks up", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(dir, "apps/web/e2e/fixtures/probe-component.tsx", LEGACY_SPEC);
|
||||
stage(dir);
|
||||
|
||||
const { added, violations } = await run(dir);
|
||||
expect(added).toEqual(["apps/web/e2e/fixtures/probe-component.tsx"]);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({ line: 5, whole: true, messageId: "waitForTimeout" });
|
||||
});
|
||||
|
||||
/**
|
||||
* The finding must land on the changed line. Reporting the whole
|
||||
* `CallExpression` put it on the receiver, so converting `.click(…)` to
|
||||
* `.waitForTimeout(…)` in a wrapped chain left the finding on an UNCHANGED
|
||||
* line and the ratchet dropped it. Found in review.
|
||||
*/
|
||||
it("FAILS when only the method line of a wrapped call changes", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
const wrapped = [
|
||||
'import { test } from "@playwright/test";',
|
||||
"",
|
||||
'test("wrapped", async ({ page }) => {',
|
||||
" await page",
|
||||
' .click("#a");',
|
||||
"});",
|
||||
"",
|
||||
].join("\n");
|
||||
write(dir, SPEC, wrapped);
|
||||
git(dir, ["add", "-A"]);
|
||||
git(dir, ["commit", "-q", "-m", "wrapped chain"]);
|
||||
git(dir, ["update-ref", "refs/remotes/origin/main", "HEAD"]);
|
||||
// Only line 5 changes; line 4 (`await page`) is untouched.
|
||||
write(dir, SPEC, wrapped.replace(' .click("#a");', " .waitForTimeout(500);"));
|
||||
|
||||
const { violations } = await run(dir);
|
||||
expect(violations).toHaveLength(1);
|
||||
expect(violations[0]).toMatchObject({ line: 5, whole: false, messageId: "waitForTimeout" });
|
||||
});
|
||||
|
||||
/** Non-e2e changes are none of this gate's business. */
|
||||
it("ignores changes outside apps/web/e2e", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(dir, "apps/web/components/card.tsx", "export const C = () => null;\n");
|
||||
write(dir, "apps/backend/main.go", "package main\n");
|
||||
|
||||
const { added, modified, violations } = await run(dir);
|
||||
expect(added).toEqual([]);
|
||||
expect(modified).toEqual([]);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* `causal-waits.ts` implements both banned forms. If the exemption ever stops
|
||||
* applying, the helper the whole effort promotes becomes unlandable.
|
||||
*/
|
||||
it("exempts the causal-waits helper even when added whole", async () => {
|
||||
const dir = repoWithLegacySleep();
|
||||
write(
|
||||
dir,
|
||||
"apps/web/e2e/helpers/causal-waits.ts",
|
||||
[
|
||||
"export function dwell(page: Page, ms: number): Promise<void> {",
|
||||
" return page.waitForTimeout(ms);",
|
||||
"}",
|
||||
"export function injectLatency(ms: number): Promise<void> {",
|
||||
" return new Promise((resolve) => setTimeout(resolve, ms));",
|
||||
"}",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
stage(dir);
|
||||
|
||||
const { added, violations } = await run(dir);
|
||||
expect(added).toEqual(["apps/web/e2e/helpers/causal-waits.ts"]);
|
||||
expect(violations).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* The ratchet is actually WIRED UP.
|
||||
*
|
||||
* Every other test here proves the matcher is right and the diff attribution is
|
||||
* right. None of them would notice if the CI step, the pre-commit hook or the
|
||||
* script's own call to `findNewSleeps` were deleted — the rule would still be
|
||||
* correct, its tests would still pass, and the gate would silently never run
|
||||
* again. That is the worst failure mode available to a check: it reports a clean
|
||||
* tree forever, in the one place nobody thinks to look.
|
||||
*
|
||||
* So these assert the call sites, not the logic. They are the mutants
|
||||
* "delete the CI step", "delete the hook", and "delete the call while keeping
|
||||
* the function".
|
||||
*
|
||||
* Matching is on the script FILENAME rather than on surrounding YAML structure,
|
||||
* so reformatting, renaming a step or reordering the workflow does not break
|
||||
* them — only actually removing the invocation does.
|
||||
*/
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
const WEB_DIR = path.resolve(import.meta.dirname, "../..");
|
||||
const REPO_ROOT = path.resolve(WEB_DIR, "../..");
|
||||
|
||||
const RATCHET_SCRIPT = "check-new-e2e-sleeps.mjs";
|
||||
|
||||
const read = (rel: string) => fs.readFileSync(path.join(REPO_ROOT, rel), "utf8");
|
||||
|
||||
describe("sleep ratchet wiring", () => {
|
||||
/**
|
||||
* Both invocations, not just the filename.
|
||||
*
|
||||
* `toContain(RATCHET_SCRIPT)` alone was too weak: the step has a
|
||||
* `pull_request` branch and a `push` branch, so deleting either one left the
|
||||
* other's mention of the filename and the assertion still passed. A mutation
|
||||
* run caught it. Count the call sites instead.
|
||||
*/
|
||||
it("runs in CI, in the frontend workflow, on both event paths", () => {
|
||||
const workflow = read(".github/workflows/frontend-tests.yml");
|
||||
expect(workflow).toMatch(/^\s+- name: Run E2E sleep ratchet$/m);
|
||||
const calls = workflow.match(new RegExp(`node scripts/${RATCHET_SCRIPT}`, "g")) ?? [];
|
||||
expect(calls).toHaveLength(2);
|
||||
});
|
||||
|
||||
/**
|
||||
* `pull_request` must NOT pass `--base`. The checkout is the merge ref and
|
||||
* `pull_request.base.sha` is an ancestor of it, so handing it in makes every
|
||||
* commit main landed since the PR opened read as this PR's new code (#2160
|
||||
* did exactly this to the i18n ratchet).
|
||||
*/
|
||||
it("does not pass --base on pull_request in CI", () => {
|
||||
const workflow = read(".github/workflows/frontend-tests.yml");
|
||||
const step = workflow.slice(workflow.indexOf("Run E2E sleep ratchet"));
|
||||
// Match the pull_request branch structurally rather than slicing at the
|
||||
// first `BASE_SHA=` literal: a future YAML comment mentioning that name
|
||||
// would move the slice boundary and quietly shrink what is asserted
|
||||
// (review finding). This is strictly tighter than the slice was — it pins
|
||||
// the invocation, its bare form, and the early exit, in order.
|
||||
expect(step).toMatch(
|
||||
new RegExp(
|
||||
`if \\[\\[ "\\$\\{\\{ github.event_name \\}\\}" == "pull_request" \\]\\];? then\\n` +
|
||||
`\\s+node scripts/${RATCHET_SCRIPT}\\n` +
|
||||
`\\s+exit 0\\n`,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The hook id is matched to end-of-line. `toContain("id: e2e-sleep-ratchet")`
|
||||
* was satisfied by `id: e2e-sleep-ratchet-disabled`, so renaming the hook to
|
||||
* switch it off passed the test — a mutation run caught it.
|
||||
*/
|
||||
it("runs as a pre-commit hook on e2e changes", () => {
|
||||
const config = read(".pre-commit-config.yaml");
|
||||
expect(config).toMatch(/^\s+- id: e2e-sleep-ratchet$/m);
|
||||
expect(config).toContain(RATCHET_SCRIPT);
|
||||
// The hook must fire for the tree it guards.
|
||||
expect(config).toContain("^apps/web/e2e/.*\\.tsx?$");
|
||||
});
|
||||
|
||||
it("is reachable as a package script", () => {
|
||||
const pkg = JSON.parse(read("apps/web/package.json"));
|
||||
expect(pkg.scripts["e2e:sleep-ratchet"]).toContain(RATCHET_SCRIPT);
|
||||
expect(pkg.scripts["e2e:waits"]).toContain("report-e2e-waits.mjs");
|
||||
expect(pkg.scripts["lint:e2e-sleeps"]).toContain("eslint.e2e-sleeps.config.mjs");
|
||||
});
|
||||
|
||||
/**
|
||||
* The CLI must actually invoke the decision function. A shell that imports it
|
||||
* and then exits 0 unconditionally is the "delete the call, keep the function"
|
||||
* mutant, and it looks completely healthy from the outside.
|
||||
*/
|
||||
it("the CLI calls findNewSleeps and exits non-zero on violations", () => {
|
||||
const cli = read(`apps/web/scripts/${RATCHET_SCRIPT}`);
|
||||
expect(cli).toContain("await findNewSleeps(");
|
||||
expect(cli).toContain("process.exit(1)");
|
||||
// The failure exit must be reachable from the violations branch, not
|
||||
// orphaned below an unconditional success exit.
|
||||
expect(cli.indexOf("violations.length === 0")).toBeLessThan(cli.indexOf("process.exit(1)"));
|
||||
});
|
||||
|
||||
/**
|
||||
* The eslint rule is registered in the main config too, on the directories
|
||||
* already converted. Losing this registration removes editor feedback and the
|
||||
* graduation path, without any test elsewhere noticing.
|
||||
*/
|
||||
it("registers the rule in the main eslint config", () => {
|
||||
const config = read("apps/web/eslint.config.mjs");
|
||||
expect(config).toContain("e2eSleepGuardFiles");
|
||||
expect(config).toContain('"e2e-sleeps/no-unsanctioned-sleep": "error"');
|
||||
});
|
||||
|
||||
/**
|
||||
* The guard list may only grow. A directory listed here was measured at zero,
|
||||
* so removing an entry means somebody added a sleep to a clean directory and
|
||||
* deleted the guard rather than the sleep.
|
||||
*/
|
||||
it("keeps every seeded guard directory", () => {
|
||||
const config = read("apps/web/eslint-rules/no-unsanctioned-sleep.mjs");
|
||||
for (const dir of [
|
||||
"e2e/scripts/**/*.{ts,tsx}",
|
||||
"e2e/tests/cli-mode/**/*.{ts,tsx}",
|
||||
"e2e/tests/docker/**/*.{ts,tsx}",
|
||||
"e2e/tests/github/**/*.{ts,tsx}",
|
||||
"e2e/tests/i18n/**/*.{ts,tsx}",
|
||||
"e2e/tests/kanban/**/*.{ts,tsx}",
|
||||
"e2e/tests/preview/**/*.{ts,tsx}",
|
||||
]) {
|
||||
expect(config).toContain(`"${dir}"`);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,152 @@
|
||||
/**
|
||||
* Inventory of the *sanctioned* waits in `apps/web/e2e` — the debt report's
|
||||
* data layer.
|
||||
*
|
||||
* Counting raw sleeps is not this module's job: those come from the eslint rule
|
||||
* (`eslint-rules/no-unsanctioned-sleep.mjs`), so there is exactly one definition
|
||||
* of what a banned sleep is. This module only reads the two legal forms, which
|
||||
* the rule never sees.
|
||||
*
|
||||
* ## What the numbers mean
|
||||
*
|
||||
* `dwell` carries a category, and the categories are not interchangeable:
|
||||
*
|
||||
* - **`negative-assertion` is permanent and is NOT debt.** A test asserting
|
||||
* that something never happens has no event to wait for. "Fixing" one by
|
||||
* deleting the wait removes the window in which the regression could occur
|
||||
* and leaves a test that passes for free. It is reported separately so
|
||||
* nobody optimises it toward zero.
|
||||
* - **`unverified` is the debt to drive to zero.** It means the author could
|
||||
* not name the timer. Every other category names one.
|
||||
* - The five named-timer categories are compromises of varying fixability,
|
||||
* reported so a conversion effort can pick targets.
|
||||
*
|
||||
* `injectLatency` is counted entirely separately. It delays the system under
|
||||
* test on purpose — the delay is the stimulus, and removing it destroys the
|
||||
* scenario. Folding it into a sleep total would let deliberate fixture
|
||||
* configuration inflate a number that is supposed to be shrinking.
|
||||
*/
|
||||
// `typescript-eslint`, not `@typescript-eslint/typescript-estree` directly:
|
||||
// pnpm's strict layout does not hoist the transitive package, so importing it
|
||||
// fails with ERR_MODULE_NOT_FOUND. The umbrella package is a declared dependency
|
||||
// and re-exports the same parser.
|
||||
import tseslint from "typescript-eslint";
|
||||
|
||||
/**
|
||||
* The category list, read out of `causal-waits.ts` itself rather than restated.
|
||||
*
|
||||
* A restated copy drifts silently: a category added to the helper would be
|
||||
* counted as unknown here, and the report would under-count the very thing it
|
||||
* exists to track. Throws rather than falling back to a hardcoded list, because
|
||||
* a silently-empty category set makes every dwell read as uncategorised.
|
||||
*/
|
||||
export function readDwellCategories(source) {
|
||||
const match = /export const DWELL_CATEGORIES = \[([\s\S]*?)\] as const;/.exec(source);
|
||||
if (!match) {
|
||||
throw new Error(
|
||||
"e2e-wait-inventory: could not find `export const DWELL_CATEGORIES = [...] as const;` in " +
|
||||
"causal-waits.ts. The report derives categories from the helper so the two cannot drift; " +
|
||||
"update this parser rather than hardcoding a list.",
|
||||
);
|
||||
}
|
||||
const categories = [...match[1].matchAll(/"([a-z-]+)"/g)].map((entry) => entry[1]);
|
||||
if (categories.length === 0) {
|
||||
throw new Error("e2e-wait-inventory: DWELL_CATEGORIES parsed to an empty list");
|
||||
}
|
||||
return categories;
|
||||
}
|
||||
|
||||
function walk(node, visit) {
|
||||
if (!node || typeof node.type !== "string") return;
|
||||
visit(node);
|
||||
for (const key of Object.keys(node)) {
|
||||
if (key === "parent") continue;
|
||||
const value = node[key];
|
||||
if (Array.isArray(value)) {
|
||||
for (const child of value) walk(child, visit);
|
||||
} else if (value && typeof value === "object") {
|
||||
walk(value, visit);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function calleeName(node) {
|
||||
return node.callee?.type === "Identifier" ? node.callee.name : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The category argument of a `dwell()` call.
|
||||
*
|
||||
* `dwell(ms, category, reason)` and `dwell(page, ms, category, reason)` are
|
||||
* overloads on one name, so the category sits at index 1 or 2 depending on the
|
||||
* form. It is always the FIRST string-literal argument either way — `ms` is
|
||||
* numeric and `page` is an identifier — and `reason` is the second. Matching by
|
||||
* position would need the two forms told apart; matching the first string does
|
||||
* not, and `dwell` validates the value at runtime against the same list, so an
|
||||
* unrecognised one here is a real miscategorisation rather than a parse gap.
|
||||
*/
|
||||
function dwellCategory(node, categories) {
|
||||
const strings = node.arguments.filter(
|
||||
(argument) => argument.type === "Literal" && typeof argument.value === "string",
|
||||
);
|
||||
const category = strings[0]?.value;
|
||||
if (category === undefined) return "«no literal category»";
|
||||
return categories.includes(category) ? category : `«unknown: ${category}»`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanctioned waits in one file.
|
||||
*
|
||||
* @returns {{ dwells: Array<{category: string, line: number}>, injectLatency: number }}
|
||||
*/
|
||||
export function inventorySource(source, filePath, categories) {
|
||||
const { ast } = tseslint.parser.parseForESLint(source, { loc: true, range: true });
|
||||
const dwells = [];
|
||||
let injectLatency = 0;
|
||||
|
||||
walk(ast, (node) => {
|
||||
if (node.type !== "CallExpression") return;
|
||||
const name = calleeName(node);
|
||||
if (name === "dwell") {
|
||||
dwells.push({ category: dwellCategory(node, categories), line: node.loc.start.line });
|
||||
} else if (name === "injectLatency") {
|
||||
injectLatency += 1;
|
||||
}
|
||||
});
|
||||
|
||||
return { filePath, dwells, injectLatency };
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold per-file inventories into the report's totals.
|
||||
*
|
||||
* `negativeAssertion` is excluded from `debt` on purpose — see the module
|
||||
* comment. `debt` is the number a conversion effort is trying to move.
|
||||
*/
|
||||
export function summarize(inventories, categories) {
|
||||
const byCategory = Object.fromEntries(categories.map((category) => [category, 0]));
|
||||
let unknown = 0;
|
||||
let injectLatency = 0;
|
||||
|
||||
for (const inventory of inventories) {
|
||||
injectLatency += inventory.injectLatency;
|
||||
for (const dwell of inventory.dwells) {
|
||||
if (dwell.category in byCategory) byCategory[dwell.category] += 1;
|
||||
else unknown += 1;
|
||||
}
|
||||
}
|
||||
|
||||
const permanent = byCategory["negative-assertion"] ?? 0;
|
||||
const total = Object.values(byCategory).reduce((sum, count) => sum + count, 0) + unknown;
|
||||
|
||||
return {
|
||||
byCategory,
|
||||
unknown,
|
||||
injectLatency,
|
||||
totalDwells: total,
|
||||
permanent,
|
||||
// Everything that is a compromise rather than a fact of the test.
|
||||
debt: total - permanent,
|
||||
unverified: byCategory.unverified ?? 0,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,163 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { inventorySource, readDwellCategories, summarize } from "./e2e-wait-inventory.mjs";
|
||||
|
||||
const CAUSAL_WAITS = path.resolve(import.meta.dirname, "../../e2e/helpers/causal-waits.ts");
|
||||
const CATEGORIES = readDwellCategories(fs.readFileSync(CAUSAL_WAITS, "utf8")) as string[];
|
||||
|
||||
/** Extracted because it is the one category that is permanent rather than debt. */
|
||||
const NEGATIVE = "negative-assertion";
|
||||
|
||||
const inventory = (code: string) => inventorySource(code, "fixture.ts", CATEGORIES);
|
||||
|
||||
describe("readDwellCategories", () => {
|
||||
/**
|
||||
* Read from the helper rather than restated here. If `causal-waits.ts` grows a
|
||||
* category, this picks it up; if the export is renamed or reshaped, the parse
|
||||
* throws instead of silently reporting every dwell as uncategorised.
|
||||
*/
|
||||
it("reads the live category list out of causal-waits.ts", () => {
|
||||
expect(CATEGORIES).toEqual([
|
||||
NEGATIVE,
|
||||
"product-timer",
|
||||
"library-timer",
|
||||
"clock-separation",
|
||||
"poll-interval",
|
||||
"browser-chrome",
|
||||
"unverified",
|
||||
]);
|
||||
});
|
||||
|
||||
it("throws rather than guessing when the export cannot be found", () => {
|
||||
expect(() => readDwellCategories("export const SOMETHING_ELSE = [];")).toThrow(
|
||||
/could not find/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("throws when the list parses to nothing", () => {
|
||||
expect(() => readDwellCategories("export const DWELL_CATEGORIES = [] as const;")).toThrow(
|
||||
/empty list/i,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("inventorySource", () => {
|
||||
/**
|
||||
* The two overloads put `category` at a different index — 1 without a page, 2
|
||||
* with one. Both must land in the same bucket, or the report splits one
|
||||
* category across two rows depending on which form the author used.
|
||||
*/
|
||||
it("reads the category from the page-less form", () => {
|
||||
const found = inventory('await dwell(500, "poll-interval", "backend health poll");');
|
||||
expect(found.dwells).toEqual([{ category: "poll-interval", line: 1 }]);
|
||||
});
|
||||
|
||||
it("reads the category from the page form", () => {
|
||||
const found = inventory('await dwell(page, 300, "library-timer", "Radix open delay");');
|
||||
expect(found.dwells).toEqual([{ category: "library-timer", line: 1 }]);
|
||||
});
|
||||
|
||||
it("counts every category separately across a file", () => {
|
||||
const found = inventory(`
|
||||
await dwell(page, 300, "${NEGATIVE}", "asserting the toast never appears");
|
||||
await dwell(100, "unverified", "pre-existing spacing, no timer identified");
|
||||
await dwell(page, 50, "clock-separation", "keep the two writes distinguishable");
|
||||
`);
|
||||
expect(found.dwells.map((entry: { category: string }) => entry.category)).toEqual([
|
||||
NEGATIVE,
|
||||
"unverified",
|
||||
"clock-separation",
|
||||
]);
|
||||
});
|
||||
|
||||
it("flags a category that is not in the live list rather than dropping it", () => {
|
||||
const found = inventory('await dwell(500, "made-up-category", "why");');
|
||||
expect(found.dwells[0].category).toBe("«unknown: made-up-category»");
|
||||
});
|
||||
|
||||
it("flags a dwell whose category is not a literal", () => {
|
||||
const found = inventory("await dwell(500, category, reason);");
|
||||
expect(found.dwells[0].category).toBe("«no literal category»");
|
||||
});
|
||||
|
||||
it("counts injectLatency apart from dwell", () => {
|
||||
const found = inventory(`
|
||||
await injectLatency(800, "make the spinner observable");
|
||||
await dwell(100, "unverified", "no timer identified");
|
||||
`);
|
||||
expect(found.injectLatency).toBe(1);
|
||||
expect(found.dwells).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("parses TypeScript syntax without a type checker", () => {
|
||||
const found = inventory(`
|
||||
const wait = async (page: Page): Promise<void> => {
|
||||
await dwell(page, 10, "browser-chrome", "native print dialog");
|
||||
};
|
||||
`);
|
||||
expect(found.dwells).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("summarize", () => {
|
||||
const build = (categories: string[], injectLatency = 0) => [
|
||||
{
|
||||
filePath: "fixture.ts",
|
||||
dwells: categories.map((category) => ({ category, line: 1 })),
|
||||
injectLatency,
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
* The load-bearing assertion of the whole report: `negative-assertion` is
|
||||
* permanent and must never appear in the number a conversion effort is trying
|
||||
* to reduce. A test asserting something never happens has no event to wait
|
||||
* for, and deleting its wait leaves a test that passes for free.
|
||||
*/
|
||||
it("excludes negative-assertion from debt", () => {
|
||||
const summary = summarize(build([NEGATIVE, NEGATIVE]), CATEGORIES);
|
||||
expect(summary.totalDwells).toBe(2);
|
||||
expect(summary.permanent).toBe(2);
|
||||
expect(summary.debt).toBe(0);
|
||||
});
|
||||
|
||||
it("counts every other category as debt", () => {
|
||||
const summary = summarize(
|
||||
build([NEGATIVE, "unverified", "library-timer", "poll-interval"]),
|
||||
CATEGORIES,
|
||||
);
|
||||
expect(summary.totalDwells).toBe(4);
|
||||
expect(summary.permanent).toBe(1);
|
||||
expect(summary.debt).toBe(3);
|
||||
expect(summary.unverified).toBe(1);
|
||||
});
|
||||
|
||||
/**
|
||||
* injectLatency is deliberate fixture configuration, correct by construction.
|
||||
* Folding it in would make adding a legitimate latency simulation read as a
|
||||
* regression in a number that is supposed to be shrinking.
|
||||
*/
|
||||
it("keeps injectLatency out of both dwell totals", () => {
|
||||
const summary = summarize(build(["unverified"], 5), CATEGORIES);
|
||||
expect(summary.injectLatency).toBe(5);
|
||||
expect(summary.totalDwells).toBe(1);
|
||||
expect(summary.debt).toBe(1);
|
||||
});
|
||||
|
||||
it("surfaces uncategorised dwells instead of silently dropping them", () => {
|
||||
const summary = summarize(build(["«unknown: nope»", "unverified"]), CATEGORIES);
|
||||
expect(summary.unknown).toBe(1);
|
||||
expect(summary.totalDwells).toBe(2);
|
||||
expect(summary.debt).toBe(2);
|
||||
});
|
||||
|
||||
it("reports zeros for a suite with no dwells at all", () => {
|
||||
const summary = summarize([], CATEGORIES);
|
||||
expect(summary.totalDwells).toBe(0);
|
||||
expect(summary.debt).toBe(0);
|
||||
expect(summary.byCategory.unverified).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,129 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Debt report for wall-clock waits in `apps/web/e2e`.
|
||||
*
|
||||
* **This is a report, not a gate.** It never exits non-zero on a count. The
|
||||
* conversion effort is actively *adding* `unverified` dwells as it replaces raw
|
||||
* sleeps with categorised ones — that is progress, because an `unverified`
|
||||
* dwell states its ignorance where a bare `waitForTimeout(500)` hid it — so a
|
||||
* ratchet on the number would fight the migration it exists to measure. The
|
||||
* gate is `check-new-e2e-sleeps.mjs`, and it gates the *form*, not the count.
|
||||
*
|
||||
* Three numbers, deliberately kept apart:
|
||||
*
|
||||
* - **raw sleeps**, counted by the same eslint rule the gate uses, so the
|
||||
* report and the gate cannot disagree about what a sleep is. This is what
|
||||
* the conversion drives to zero, and reaching zero is what makes the
|
||||
* graduation flip (rule to `"error"` on all of `e2e/**`) possible.
|
||||
* - **dwell debt**, every categorised wait EXCEPT `negative-assertion`.
|
||||
* `unverified` is called out inside it as the sub-total to attack first.
|
||||
* - **injectLatency**, counted on its own. It is deliberate fixture
|
||||
* configuration, correct by construction, and must never be converted; if
|
||||
* it were folded into the sleep total, adding a legitimate latency
|
||||
* simulation would look like a regression.
|
||||
*
|
||||
* `negative-assertion` is printed under a heading that says it is permanent.
|
||||
* A test asserting something never happens has no event to wait on, and
|
||||
* deleting its wait removes the window the regression needs in order to appear.
|
||||
*
|
||||
* Usage: node scripts/report-e2e-waits.mjs [--json]
|
||||
*/
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
import { ESLint } from "eslint";
|
||||
|
||||
import { SLEEP_EXEMPT_FILES, SLEEP_RULE_ID } from "../eslint-rules/no-unsanctioned-sleep.mjs";
|
||||
import { inventorySource, readDwellCategories, summarize } from "./lib/e2e-wait-inventory.mjs";
|
||||
import { WEB_DIR } from "./lib/git-base.mjs";
|
||||
|
||||
const E2E_DIR = path.join(WEB_DIR, "e2e");
|
||||
const CAUSAL_WAITS = path.join(E2E_DIR, "helpers", "causal-waits.ts");
|
||||
|
||||
function collectTsFiles(dir) {
|
||||
const out = [];
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
if (entry.name === "node_modules" || entry.name === "test-results") continue;
|
||||
out.push(...collectTsFiles(full));
|
||||
} else if (entry.name.endsWith(".ts")) {
|
||||
out.push(full);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const categories = readDwellCategories(fs.readFileSync(CAUSAL_WAITS, "utf8"));
|
||||
const files = collectTsFiles(E2E_DIR);
|
||||
|
||||
// The helper's own definitions and its unit test would otherwise be counted as
|
||||
// usage sites, inflating every category by the examples that define it. Same
|
||||
// list the eslint configs use, so "exempt from the rule" and "not a usage site"
|
||||
// cannot drift apart.
|
||||
const exempt = new Set(SLEEP_EXEMPT_FILES.map((file) => path.join(WEB_DIR, file)));
|
||||
|
||||
// Sanctioned waits: our own AST walk, since the rule never sees them.
|
||||
const inventories = files
|
||||
.filter((file) => !exempt.has(file))
|
||||
.map((file) => inventorySource(fs.readFileSync(file, "utf8"), file, categories));
|
||||
const dwell = summarize(inventories, categories);
|
||||
|
||||
// Raw sleeps: the gate's own rule, so there is one definition of "a sleep".
|
||||
const eslint = new ESLint({
|
||||
cwd: WEB_DIR,
|
||||
overrideConfigFile: path.join(WEB_DIR, "eslint.e2e-sleeps.config.mjs"),
|
||||
warnIgnored: false,
|
||||
});
|
||||
const results = await eslint.lintFiles(files);
|
||||
|
||||
const raw = { waitForTimeout: 0, promiseSleep: 0, byFile: {} };
|
||||
for (const result of results) {
|
||||
const messages = result.messages.filter((message) => message.ruleId === SLEEP_RULE_ID);
|
||||
if (messages.length === 0) continue;
|
||||
raw.byFile[path.relative(WEB_DIR, result.filePath)] = messages.length;
|
||||
for (const message of messages) {
|
||||
if (message.messageId === "waitForTimeout") raw.waitForTimeout += 1;
|
||||
else raw.promiseSleep += 1;
|
||||
}
|
||||
}
|
||||
const rawTotal = raw.waitForTimeout + raw.promiseSleep;
|
||||
|
||||
if (process.argv.includes("--json")) {
|
||||
console.log(JSON.stringify({ raw: { ...raw, total: rawTotal }, dwell }, null, 2));
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const pad = (label) => label.padEnd(22);
|
||||
console.log("\nE2E wall-clock wait inventory\n");
|
||||
|
||||
console.log(` UNSANCTIONED — drive to zero, then graduate the rule to "error"`);
|
||||
console.log(` ${pad("waitForTimeout")}${raw.waitForTimeout}`);
|
||||
console.log(` ${pad("promise sleep")}${raw.promiseSleep}`);
|
||||
console.log(` ${pad("total")}${rawTotal}`);
|
||||
|
||||
console.log(`\n DWELL DEBT — categorised, ${dwell.debt} of ${dwell.totalDwells} are debt`);
|
||||
for (const category of categories) {
|
||||
if (category === "negative-assertion") continue;
|
||||
const count = dwell.byCategory[category] ?? 0;
|
||||
const note = category === "unverified" ? " <- attack this first" : "";
|
||||
console.log(` ${pad(category)}${count}${note}`);
|
||||
}
|
||||
if (dwell.unknown > 0) {
|
||||
console.log(` ${pad("uncategorised")}${dwell.unknown} <- miscategorised or unparsed`);
|
||||
}
|
||||
|
||||
console.log(`\n PERMANENT — not debt, do not drive down`);
|
||||
console.log(` ${pad("negative-assertion")}${dwell.permanent}`);
|
||||
|
||||
console.log(`\n DELIBERATE STIMULUS — not a wait, counted apart from sleep debt`);
|
||||
console.log(` ${pad("injectLatency")}${dwell.injectLatency}`);
|
||||
|
||||
if (rawTotal > 0) {
|
||||
const worst = Object.entries(raw.byFile)
|
||||
.sort(([, a], [, b]) => b - a)
|
||||
.slice(0, 10);
|
||||
console.log(`\n Top files by remaining raw sleeps (${Object.keys(raw.byFile).length} total):`);
|
||||
for (const [file, count] of worst) console.log(` ${String(count).padStart(3)} ${file}`);
|
||||
}
|
||||
console.log("");
|
||||
Reference in New Issue
Block a user