test(gates): the snapshot holds a config per scope, not a path per file
gates.snapshot.json listed ~800 file paths under their config, so every new or deleted frontend file regenerated it (31% of commits, E34 S2) and the regen diff buried the gate changes it exists to show. Now a scope is named by path (scopeOf: src, src tests, ui, lib tests, …), the snapshot holds one effective config per scope (2345 → 1456 lines), and a new test fails when a file's config is not its scope's. Not narrowed: a new file in a known place changes nothing (it gets its scope's config, checked), a glob that moves one file into another config fails (seen: an override on Feed.tsx → "src: src/components/Feed.tsx"), a scope whose rules change is a snapshot diff, and a file no config lints is still a register miss.
This commit is contained in:
@@ -101,20 +101,23 @@ function restrictedEntries(entry: Linter.RuleEntry | undefined): Set<string> {
|
||||
|
||||
// --- the snapshot ---------------------------------------------------------------------------------
|
||||
// The configs that can narrow a gate in more shapes than a check can enumerate are held to a
|
||||
// COMMITTED SNAPSHOT (E32 review): ESLint's effective config for every file `eslint .` reaches, and
|
||||
// the lines that run the lanes. Any change fails until the snapshot is regenerated, so a narrowing
|
||||
// arrives as a diff of `gates.snapshot.json` in review instead of slipping past a detector. (Three
|
||||
// review rounds kept finding shapes the detector this replaced did not model — a `basePath` global
|
||||
// ignore, a widened glob filed under a narrow row, a substring check on a gate line. A comparison
|
||||
// against everything has no shape to miss.)
|
||||
// COMMITTED SNAPSHOT (E32 review): ESLint's effective config per scope, and the lines that run the
|
||||
// lanes. Any change fails until the snapshot is regenerated, so a narrowing arrives as a diff of
|
||||
// `gates.snapshot.json` in review instead of slipping past a detector. (Three review rounds kept
|
||||
// finding shapes the detector this replaced did not model — a `basePath` global ignore, a widened glob
|
||||
// filed under a narrow row, a substring check on a gate line. A comparison against everything has no
|
||||
// shape to miss.)
|
||||
//
|
||||
// A scope is named by PATH (`scopeOf`), and every file of a scope must get the same effective config.
|
||||
// So a new file in a known place changes nothing (until E35 the snapshot listed every file, and 31% of
|
||||
// commits regenerated it), while a glob that moves a file into another config splits its scope and
|
||||
// fails below, and a scope whose rules change is a snapshot diff.
|
||||
//
|
||||
// Regenerate: GATE_SNAPSHOT=update npx vitest run src/lib/gates.contract.test.ts
|
||||
// then read the diff like any gate change. An ESLint config that changed keeps its scope name when
|
||||
// most of its files had it; one no name fits is written as `new config N` — rename it to the
|
||||
// register scope it is (the register test below then asks for its rows).
|
||||
// then read the diff like any gate change. A new kind of file (a new config or a new place) needs a
|
||||
// line in `scopeOf` and, when it narrows anything, its register rows.
|
||||
|
||||
const SNAPSHOT = "frontend/src/lib/gates.snapshot.json";
|
||||
const NOT_LINTED = "(not linted)";
|
||||
type EslintConfig = {
|
||||
rules: Record<string, unknown[]>;
|
||||
linterOptions: Record<string, unknown>;
|
||||
@@ -123,10 +126,24 @@ type EslintConfig = {
|
||||
settings: unknown;
|
||||
};
|
||||
type Snapshot = {
|
||||
eslint: { configs: Record<string, EslintConfig>; files: Record<string, string[]> };
|
||||
eslint: Record<string, EslintConfig>;
|
||||
lanes: Record<string, unknown>;
|
||||
};
|
||||
|
||||
/** The register scope a code file belongs to, by its path (relative to frontend/). */
|
||||
function scopeOf(file: string): string {
|
||||
if (!file.includes("/")) return /[.][mc]?js$/.test(file) ? "js configs" : "ts configs";
|
||||
if (file.startsWith("e2e/")) return "e2e";
|
||||
if (file.startsWith("src/design-lab/")) return "design-lab";
|
||||
// the network owners (NETWORK_OWNERS in eslint.config.js)
|
||||
if (file === "src/lib/http.ts") return "lib/http.ts";
|
||||
if (file === "src/lib/api/core.ts") return "lib/api/core.ts";
|
||||
const test = /[.]test[.]tsx?$/.test(file);
|
||||
if (file.startsWith("src/components/ui/")) return test ? "ui tests" : "ui";
|
||||
if (file.startsWith("src/lib/")) return test ? "lib tests" : "src/lib";
|
||||
return test ? "src tests" : "src";
|
||||
}
|
||||
|
||||
const sortKeys = (v: unknown): unknown =>
|
||||
Array.isArray(v)
|
||||
? v.map(sortKeys)
|
||||
@@ -173,7 +190,7 @@ function normalize(cfg: Linter.Config): EslintConfig {
|
||||
|
||||
// Every code file `eslint .` walks into. The skipped directories are the global ignores' generated
|
||||
// output (the register's `everywhere` row) — untracked, and present on one machine but not another.
|
||||
// Every other file is asked for its config, so a file a new ignore drops lands in `(not linted)`.
|
||||
// Every other file is asked for its config, so a file a new ignore drops is reported as not linted.
|
||||
const WALK_SKIP = new Set([
|
||||
"node_modules",
|
||||
"dist",
|
||||
@@ -192,7 +209,7 @@ function codeFiles(dir = ""): string[] {
|
||||
|
||||
// On disk every config but src is written as its difference from src — the rules it sets otherwise
|
||||
// and the src rules it does not apply — so a scope reads like its register rows, and one rule set
|
||||
// is not repeated fourteen times. In memory it is always whole.
|
||||
// is not repeated a dozen times. In memory it is always whole.
|
||||
type Stored = Omit<EslintConfig, "rules"> & {
|
||||
rules: Record<string, unknown[]>;
|
||||
notApplied?: string[];
|
||||
@@ -225,15 +242,22 @@ function fromStored(stored: Record<string, Stored>): Record<string, EslintConfig
|
||||
);
|
||||
}
|
||||
const committed = (): Snapshot => {
|
||||
const s = JSON.parse(read(SNAPSHOT)) as Snapshot & {
|
||||
eslint: { configs: Record<string, Stored> };
|
||||
const s = JSON.parse(read(SNAPSHOT)) as Omit<Snapshot, "eslint"> & {
|
||||
eslint: Record<string, Stored>;
|
||||
};
|
||||
return canonical({ ...s, eslint: { ...s.eslint, configs: fromStored(s.eslint.configs) } });
|
||||
return canonical({ ...s, eslint: fromStored(s.eslint) });
|
||||
};
|
||||
|
||||
async function eslintActual(names: Snapshot["eslint"]): Promise<Snapshot["eslint"]> {
|
||||
type EslintActual = {
|
||||
configs: Record<string, EslintConfig>;
|
||||
/** the files no config lints */
|
||||
notLinted: string[];
|
||||
/** `scope: file` for each file whose config is not its scope's (the scope's majority) */
|
||||
strays: string[];
|
||||
};
|
||||
async function eslintActual(): Promise<EslintActual> {
|
||||
const eslint = new ESLint({ cwd: FRONTEND });
|
||||
const groups = new Map<string, { config: EslintConfig; files: string[] }>();
|
||||
const byScope = new Map<string, Map<string, { config: EslintConfig; files: string[] }>>();
|
||||
const notLinted: string[] = [];
|
||||
for (const file of codeFiles().sort()) {
|
||||
const raw = (await eslint.calculateConfigForFile(file)) as Linter.Config | undefined;
|
||||
@@ -243,35 +267,22 @@ async function eslintActual(names: Snapshot["eslint"]): Promise<Snapshot["eslint
|
||||
}
|
||||
const config = normalize(raw);
|
||||
const key = JSON.stringify(config);
|
||||
const scope = scopeOf(file);
|
||||
const groups =
|
||||
byScope.get(scope) ?? new Map<string, { config: EslintConfig; files: string[] }>();
|
||||
const group = groups.get(key) ?? { config, files: [] };
|
||||
group.files.push(file);
|
||||
groups.set(key, group);
|
||||
byScope.set(scope, groups);
|
||||
}
|
||||
// a config is named by the snapshot entry it equals; a changed one by the old name most of its
|
||||
// files had (if still free), so the diff shows the rule that moved, not a renamed scope
|
||||
const byContent = new Map(Object.entries(names.configs).map(([n, c]) => [JSON.stringify(c), n]));
|
||||
const oldName = new Map(
|
||||
Object.entries(names.files).flatMap(([n, files]) => files.map((f) => [f, n] as const)),
|
||||
);
|
||||
const configs: Record<string, EslintConfig> = {};
|
||||
const files: Record<string, string[]> = { [NOT_LINTED]: notLinted };
|
||||
const unnamed: { config: EslintConfig; files: string[] }[] = [];
|
||||
for (const [key, group] of groups) {
|
||||
const name = byContent.get(key);
|
||||
if (name) [configs[name], files[name]] = [group.config, group.files];
|
||||
else unnamed.push(group);
|
||||
const strays: string[] = [];
|
||||
for (const [scope, groups] of byScope) {
|
||||
const [first, ...rest] = [...groups.values()].sort((a, b) => b.files.length - a.files.length);
|
||||
configs[scope] = first!.config;
|
||||
for (const g of rest) strays.push(...g.files.map((f) => `${scope}: ${f}`));
|
||||
}
|
||||
unnamed.forEach((group, i) => {
|
||||
const votes = new Map<string, number>();
|
||||
for (const f of group.files) {
|
||||
const n = oldName.get(f);
|
||||
if (n && n !== NOT_LINTED) votes.set(n, (votes.get(n) ?? 0) + 1);
|
||||
}
|
||||
const best = [...votes].sort((a, b) => b[1] - a[1])[0]?.[0];
|
||||
const name = best && !(best in configs) ? best : `new config ${i + 1}`;
|
||||
[configs[name], files[name]] = [group.config, group.files];
|
||||
});
|
||||
return canonical({ configs, files });
|
||||
return { configs: canonical(configs), notLinted, strays };
|
||||
}
|
||||
|
||||
// the lines that run each lane, comments and blank lines aside
|
||||
@@ -295,10 +306,10 @@ const lanes = () => ({
|
||||
"frontend/.prettierignore": meaningful("frontend/.prettierignore"),
|
||||
});
|
||||
|
||||
let actual: Promise<Snapshot> | undefined;
|
||||
const current = () =>
|
||||
(actual ??= (async () =>
|
||||
canonical({ eslint: await eslintActual(committed().eslint), lanes: lanes() }))());
|
||||
let actual: Promise<EslintActual> | undefined;
|
||||
const linted = () => (actual ??= eslintActual());
|
||||
const current = async (): Promise<Snapshot> =>
|
||||
canonical({ eslint: (await linted()).configs, lanes: lanes() });
|
||||
|
||||
// --- the register, read off the snapshot ------------------------------------------------------------
|
||||
|
||||
@@ -310,11 +321,8 @@ const ignorable = (rule: string, all: Record<string, unknown>) =>
|
||||
const entry = (v: unknown[] | undefined) => v as Linter.RuleEntry | undefined;
|
||||
|
||||
/** Every narrowing in the effective ESLint configs, as (scope, id). */
|
||||
function eslintNarrowings({ configs, files }: Snapshot["eslint"]): [string, string][] {
|
||||
const found: [string, string][] = files[NOT_LINTED]!.map((f) => [
|
||||
"everywhere",
|
||||
`${f} — not linted`,
|
||||
]);
|
||||
function eslintNarrowings({ configs, notLinted }: EslintActual): [string, string][] {
|
||||
const found: [string, string][] = notLinted.map((f) => ["everywhere", `${f} — not linted`]);
|
||||
const src = configs.src?.rules;
|
||||
if (!src) return [...found, ["src", "(no config named src)"]];
|
||||
for (const [scope, { rules, linterOptions }] of Object.entries(configs)) {
|
||||
@@ -347,22 +355,29 @@ function eslintNarrowings({ configs, files }: Snapshot["eslint"]): [string, stri
|
||||
}
|
||||
|
||||
describe("the gate snapshot (gates.snapshot.json)", () => {
|
||||
it("holds the effective ESLint config of every file and the lines that run each lane", async () => {
|
||||
it("holds the effective ESLint config of every scope and the lines that run each lane", async () => {
|
||||
const now = await current();
|
||||
if (process.env.GATE_SNAPSHOT === "update") {
|
||||
const path = REPO + SNAPSHOT;
|
||||
const options = { ...(await resolveConfig(path)), filepath: path };
|
||||
const stored = { ...now, eslint: { ...now.eslint, configs: toStored(now.eslint.configs) } };
|
||||
const stored = { ...now, eslint: toStored(now.eslint) };
|
||||
writeFileSync(path, await format(JSON.stringify(sortKeys(stored)), options));
|
||||
return;
|
||||
}
|
||||
expect(now).toEqual(committed());
|
||||
}, 120_000);
|
||||
|
||||
// A glob that moves one file into another config (a product file into the test config) leaves its
|
||||
// scope's config as it was, so the snapshot alone would not see it: the file now differs from the
|
||||
// rest of its scope, and that is what fails here.
|
||||
it("gives every file of a scope the same config", async () => {
|
||||
expect((await linted()).strays).toEqual([]);
|
||||
}, 120_000);
|
||||
});
|
||||
|
||||
describe("the gate register (docs/gates.md)", () => {
|
||||
it("lists every ESLint rule narrowed, by scope", async () => {
|
||||
expect(missingFrom(eslintNarrowings((await current()).eslint))).toEqual([]);
|
||||
expect(missingFrom(eslintNarrowings(await linted()))).toEqual([]);
|
||||
}, 120_000);
|
||||
|
||||
it("keeps --max-warnings 0, which is what makes a warn-level rule fail", () => {
|
||||
|
||||
+1264
-2060
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user