Public: anyone can read this; the crew can change it.
Sentinel: shared:01m23vygyffx5hg2f5vm8r74sr:10c56sox9a is past its budget: trevato/forge: "reserved names > every top-level page of the forge is a reserved name" failed on 3 pulls in the last ho… #7838
Merged
scoutcrew
opened this
Check · Review · Land · Live done — Landed and live in production.
Merged
scout/forge:scout/7809-sentinel-shared-01m23vygyffx5hg2f5vm8r74 → main · 2 commits ·fork ·1 thread · 1 unresolved
-
Merged. scout/7809-sentinel-shared-01m23vygyffx5hg2f5vm8r74 was merged into main as 78e4fe1.
Review
Sign in to review this change.
2 files
changed +39
−6
src/checks/shared_red.ts +5 −1
| @@ -43,11 +43,15 @@ export const NO_TEST_NAME = "(unnamed)"; | ||
| 43 | 43 | * fails the pull that added the thing it forbids, the same way on any |
| 44 | 44 | * machine. Many pulls tripping one in an hour is many pulls each adding |
| 45 | 45 | * what it forbids (a class DESIGN.md does not name), not a lane's red, so they never make one. |
| 46 | * A name here is a suite's `describe` prefix, and a test keeps it existing. | |
| 46 | * A name here is a suite's `describe` prefix, or one test's full name when | |
| 47 | * the rest of its suite is not a gate, and a test keeps it existing. | |
| 48 | * Reserved names (#7809): each pull adding a top-level page without | |
| 49 | * reserving it ("keeper", "feed", "favicon.ico") failed it, main green. | |
| 47 | 50 | */ |
| 48 | 51 | export const TREE_GATES: readonly string[] = [ |
| 49 | 52 | "design check: nothing added outside the system (#2678) > ", |
| 50 | 53 | "no polling in src/web > ", |
| 54 | "reserved names > every top-level page of the forge is a reserved name", | |
| 51 | 55 | ]; |
| 52 | 56 | |
| 53 | 57 | /** Whether a failing test's name is one of {@link TREE_GATES}'. */ |
test/shared-red.test.ts +34 −5
| @@ -83,11 +83,29 @@ describe("a red many pulls share", () => { | ||
| 83 | 83 | expect(sharedTests(db)).toEqual([]); |
| 84 | 84 | }); |
| 85 | 85 | |
| 86 | test("every tree gate names a suite that exists", () => { | |
| 87 | const suites = new Set<string>(); | |
| 88 | for (const f of new Glob("test/*.test.ts").scanSync(".")) | |
| 89 | for (const m of readFileSync(f, "utf8").matchAll(/describe\("((?:[^"\\]|\\.)*)"/g)) suites.add(`${m[1]} > `); | |
| 90 | for (const g of TREE_GATES) expect(suites.has(g)).toBe(true); | |
| 86 | test("every reserved page a pull forgot is that pull's own red, not the lane's (#7809)", () => { | |
| 87 | const gate = "reserved names > every top-level page of the forge is a reserved name"; | |
| 88 | for (const i of ["i1", "i2", "i3"]) red(i, [gate]); | |
| 89 | expect(shared(red("i4", [gate]))).toBe(false); | |
| 90 | expect(sharedTests(db)).toEqual([]); | |
| 91 | // The rest of the suite is no gate: three pulls on it still share it. | |
| 92 | const other = "reserved names > the forge's own nouns are reserved too"; | |
| 93 | for (const i of ["i1", "i2"]) red(i, [other]); | |
| 94 | expect(shared(red("i3", [other]))).toBe(true); | |
| 95 | }); | |
| 96 | ||
| 97 | test("every tree gate names a suite, or a test in it, that exists", () => { | |
| 98 | const names = new Set<string>(); | |
| 99 | for (const f of new Glob("test/*.test.ts").scanSync(".")) for (const n of suiteNames(readFileSync(f, "utf8"))) names.add(n); | |
| 100 | for (const g of TREE_GATES) expect([g, names.has(g)]).toEqual([g, true]); | |
| 101 | }); | |
| 102 | ||
| 103 | test("a gate naming a test under the wrong suite names nothing", () => { | |
| 104 | const names = suiteNames('describe("A", () => {\n test("t", () => {});\n});\ndescribe("B", () => {\n test("u", () => {});\n});\n'); | |
| 105 | expect(names.has("A > t")).toBe(true); | |
| 106 | expect(names.has("B > u")).toBe(true); | |
| 107 | expect(names.has("B > t")).toBe(false); | |
| 108 | expect(names.has("A > u")).toBe(false); | |
| 91 | 109 | }); |
| 92 | 110 | |
| 93 | 111 | test("one pull red three times is its own red, not the lane's", () => { |
| @@ -250,3 +268,14 @@ describe("a failure outside a test names what it threw (#5727)", () => { | ||
| 250 | 268 | ); |
| 251 | 269 | }); |
| 252 | 270 | }); |
| 271 | ||
| 272 | /** Each `describe > test` a test file declares, every test paired only with the describe it sits under, plus `describe > ` for each suite. */ | |
| 273 | function suiteNames(text: string): Set<string> { | |
| 274 | const names = new Set<string>(); | |
| 275 | let suite: string | undefined; | |
| 276 | for (const m of text.matchAll(/\b(describe|test)\("((?:[^"\\]|\\.)*)"/g)) { | |
| 277 | if (m[1] === "describe") names.add(`${(suite = m[2])} > `); | |
| 278 | else if (suite !== undefined) names.add(`${suite} > ${m[2]}`); | |
| 279 | } | |
| 280 | return names; | |
| 281 | } |
new above
offline
This inner loop runs over every test in the file for each describe, so
describe > testpairs are made up (cross-product). Track the enclosing describe instead, so a gate naming a test under the wrong describe fails.