trevato/cs-engine

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

forked from trevato/cs

More
Forks3g fMapg aRunsg rEventsFlows

A test file that only passes if it is loaded second: break the packed → observe → world → bots → packed cycle #406

Merged torquecrew opened this

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

ConversationFiles
Merged

torque/cs-engine:torque/248-a-test-file-that-only-passes-if-it-is-lo → main 2 commits fork

  • Merged. torque/248-a-test-file-that-only-passes-if-it-is-lo was merged into main as 869a4c3.

Review

Sign in to review this change.
11 files changed +238 −12
README.md +11
@@ -32,6 +32,7 @@ bun run scripts/room.ts # what one room costs: C
3232 bun run scripts/room.ts --seconds 30 --margin 0.25 # a longer window, and a quarter of a core for rooms
3333 bun test # the check every pull runs
3434 bun run scripts/mutate-result.ts # break each rule in src/result.ts and src/gate.ts, see a test go red
35 bun run scripts/alone.ts # every test file in a process of its own: does any pass only in company?
3536 FORGE_APP_DATA=./data bun run src/tty.ts # the terminal interface
3637 ```
3738
@@ -820,6 +821,16 @@ never checked itself.
820821 and `verbatimModuleSyntax`. Nothing is `any`, nothing is `@ts-ignore`.
821822 - It costs about 0.6 s to typecheck the whole tree and under a second to
822823 install, once, next to a suite that takes thirteen.
824 - The suite's result does not depend on which file loads first.
825 `scripts/alone.test.ts` imports every module under `src/` and `scripts/`
826 as the first thing a fresh process loads, so a cycle that reads another
827 module's value while it initialises fails there whichever end it is entered
828 from (#248 was `packed.ts` reading `OBS_SIZE` out of `observe.ts` inside the
829 `observe` → `world` → `bots` → `packed` ring; `OBS_SIZE` now lives in
830 `src/client/obs-size.ts`, which imports nothing). `bun run scripts/alone.ts`
831 goes further and runs every test file in a process of its own, which is what
832 catches a test that leans on state another file set up. It is the whole suite
833 again, so it is a command rather than part of `bun test`.
823834
824835 ## Shape
825836
scripts/alone.probe.ts added +15
@@ -0,0 +1,15 @@
1 /**
2 * The child half of `scripts/alone.test.ts`: import one module as the first
3 * thing this process loads, and exit non-zero if loading it throws. The path
4 * arrives as an argument and is imported, never evaluated; the parent only
5 * ever sends a path its own walk found.
6 */
7
8 export {};
9
10 const file = process.argv[2];
11 if (file === undefined || !file.endsWith(".ts")) {
12 console.error("usage: bun run scripts/alone.probe.ts <module.ts>");
13 process.exit(2);
14 }
15 await import(file);
scripts/alone.test.ts added +54
@@ -0,0 +1,54 @@
1 /**
2 * Every module loads as the first thing a process imports — the load-order
3 * half of `scripts/alone.ts`, cheap enough for every push.
4 *
5 * Inside `bun test` every module is cached after its first load, so a ring
6 * entered at a harmless end once hides that it throws entered at another:
7 * `observe.ts` → `world.ts` → `bots.ts` → `packed.ts` → `observe.ts` threw
8 * `Cannot access 'OBS_SIZE' before initialization` entered at `observe.ts` or
9 * at `scripts/train.ts`, and only the order the suite loaded its files kept
10 * that quiet (#248). Importing each module alone in a fresh process enters
11 * every ring from every end, so a cycle that reads a value at load time fails
12 * here whichever file the suite happens to sort first.
13 */
14
15 import { expect, test } from "bun:test";
16 import { existsSync, mkdtempSync } from "node:fs";
17 import { tmpdir } from "node:os";
18 import { join } from "node:path";
19 import { each, ENTRIES, modules, parseJobs, ROOT, testFiles } from "./alone.ts";
20
21 const PROBE = join(import.meta.dir, "alone.probe.ts");
22
23 test("the walk finds the modules, both ends of the #248 ring among them, and leaves the entries out", () => {
24 const found = modules();
25 for (const file of ["src/client/observe.ts", "src/client/packed.ts", "src/client/obs-size.ts", "scripts/train.ts"])
26 expect(found).toContain(file);
27 for (const file of Object.keys(ENTRIES)) {
28 expect(existsSync(join(ROOT, file))).toBe(true); // an entry that is gone is a stale exemption
29 expect(found).not.toContain(file);
30 }
31 expect(found.some((file) => file.endsWith(".test.ts") || file.endsWith(".probe.ts"))).toBe(false);
32 expect(testFiles()).toContain("scripts/train.test.ts");
33 });
34
35 test("every module loads on its own, as the first thing a fresh process imports", async () => {
36 const data = mkdtempSync(join(tmpdir(), "alone-"));
37 const outcomes = await each(modules(), (file) => ["run", PROBE, join(ROOT, file)], 8, { FORGE_APP_DATA: data });
38 const failed = outcomes.filter((o) => !o.ok).map((o) => `${o.file}\n${o.output.trim().split("\n").slice(-6).join("\n")}`);
39 expect(failed).toEqual([]);
40 }, 120_000);
41
42 test("the probe refuses anything that is not a module path", async () => {
43 const [outcome] = await each(["x"], () => ["run", PROBE, "not-a-module"], 1);
44 expect(outcome!.ok).toBe(false);
45 expect(outcome!.output).toContain("usage:");
46 });
47
48 test("--jobs takes a whole number from 1 to 64 and nothing else", () => {
49 expect(parseJobs([])).toBe(8);
50 expect(parseJobs(["--jobs", "4"])).toBe(4);
51 expect(parseJobs(["--jobs", "64"])).toBe(64);
52 for (const bad of [["--jobs", "0"], ["--jobs", "65"], ["--jobs", "1.5"], ["--jobs"], ["--jobs", "-2"], ["-j", "4"]])
53 expect(parseJobs(bad)).toBeNull();
54 });
scripts/alone.ts added +130
@@ -0,0 +1,130 @@
1 /**
2 * Every file on its own: a result that depends on what loaded first is not a
3 * result. `bun test` loads every test file into one process, so a module that
4 * reads another's value while that module is still initialising (#248: the
5 * `packed.ts` → `observe.ts` → `world.ts` → `bots.ts` ring) or a test that
6 * leans on state another file set up (a bot thinking before anyone called
7 * `useShipped`) passes or fails depending on which file bun happened to load
8 * first. Run alone, each of them fails every time.
9 *
10 * bun run scripts/alone.ts # every test file, each in a process of its own
11 * bun run scripts/alone.ts --jobs 4 # four at a time rather than eight
12 *
13 * It is not part of `bun test`: every file run alone is the whole suite again,
14 * about half a minute at eight at a time, most of it `src/client/main.test.ts`.
15 * `scripts/alone.test.ts` is the part that is: it loads every module in the
16 * tree as the first thing a fresh process imports, which is the load-order
17 * half of this and costs a few seconds.
18 *
19 * Each file gets a minute a test rather than bun's five seconds: eight suites
20 * at once are slower than one, and this checks what loaded first, not how fast.
21 *
22 * The walk is here, shared by both, so a file added tomorrow is covered the
23 * day it lands. Every spawn is this bun and a path the walk found; no text from
24 * anywhere else reaches a command line.
25 */
26
27 import { Glob } from "bun";
28 import { join, resolve } from "node:path";
29
30 /** The repository root: every path below is relative to it. */
31 export const ROOT = resolve(import.meta.dir, "..");
32
33 /** Where the code lives. `node_modules/`, `dist/` and the rest are not ours. */
34 const TREES = ["src", "scripts"];
35
36 /**
37 * Files that do their work when they are loaded, and so are run rather than
38 * imported: loading one alone is starting it, not checking it. Each says why.
39 */
40 export const ENTRIES: Record<string, string> = {
41 "src/server.ts": "starts the server and opens the database",
42 "src/example/main.ts": "the example page's boot: draws into `window` on load",
43 "src/range/main.ts": "the range page's boot: draws into `window` on load",
44 "scripts/mutate-result.ts": "breaks src/result.ts on load and runs the tests against it",
45 };
46
47 /** Every `.ts` file under the trees, repository-relative with `/`, sorted. */
48 function walk(): string[] {
49 const files: string[] = [];
50 for (const tree of TREES)
51 for (const file of new Glob("**/*.ts").scanSync({ cwd: join(ROOT, tree) })) files.push(`${tree}/${file.replaceAll("\\", "/")}`);
52 return files.sort();
53 }
54
55 /** Every test file the suite runs. */
56 export const testFiles = (): string[] => walk().filter((file) => file.endsWith(".test.ts"));
57
58 /**
59 * Every module that is imported rather than run: not a test, not a test's
60 * helper, not a `.probe.ts` (tooling that runs in a child), not a declaration
61 * file and not one of the `ENTRIES`.
62 */
63 export const modules = (): string[] =>
64 walk().filter(
65 (file) =>
66 !file.endsWith(".test.ts") &&
67 !file.endsWith(".test-helper.ts") &&
68 !file.endsWith(".probe.ts") &&
69 !file.endsWith(".d.ts") &&
70 !(file in ENTRIES),
71 );
72
73 /** What one child said, and whether it exited clean. */
74 export type Outcome = { file: string; ok: boolean; output: string };
75
76 /** Run `args` for each file in a fresh bun, `jobs` at a time, in the files' order. */
77 export async function each(
78 files: string[],
79 args: (file: string) => string[],
80 jobs: number,
81 env: Record<string, string> = {},
82 ): Promise<Outcome[]> {
83 const outcomes: Outcome[] = new Array(files.length);
84 let next = 0;
85 const worker = async (): Promise<void> => {
86 while (next < files.length) {
87 const i = next++;
88 const file = files[i]!;
89 const child = Bun.spawn([process.execPath, ...args(file)], {
90 cwd: ROOT,
91 env: { ...process.env, ...env },
92 stdin: "ignore",
93 stdout: "pipe",
94 stderr: "pipe",
95 });
96 const [out, err, code] = await Promise.all([
97 new Response(child.stdout).text(),
98 new Response(child.stderr).text(),
99 child.exited,
100 ]);
101 outcomes[i] = { file, ok: code === 0, output: out + err };
102 }
103 };
104 await Promise.all(Array.from({ length: Math.max(1, Math.min(jobs, files.length)) }, worker));
105 return outcomes;
106 }
107
108 /** `--jobs N`, a whole number from 1 to 64; anything else is the usage. */
109 export function parseJobs(argv: string[]): number | null {
110 if (argv.length === 0) return 8;
111 if (argv.length !== 2 || argv[0] !== "--jobs" || !/^[0-9]{1,2}$/.test(argv[1]!)) return null;
112 const jobs = Number(argv[1]);
113 return jobs >= 1 && jobs <= 64 ? jobs : null;
114 }
115
116 if (import.meta.main) {
117 const jobs = parseJobs(process.argv.slice(2));
118 if (jobs === null) {
119 console.error("usage: bun run scripts/alone.ts [--jobs N] (N from 1 to 64, default 8)");
120 process.exit(2);
121 }
122 const files = testFiles();
123 const started = performance.now();
124 const outcomes = await each(files, (file) => ["test", "--timeout", "60000", `./${file}`], jobs);
125 const failed = outcomes.filter((o) => !o.ok);
126 for (const o of failed) console.log(`\n── ${o.file} fails on its own ──\n${o.output.trimEnd()}`);
127 const seconds = ((performance.now() - started) / 1000).toFixed(1);
128 console.log(`\n${files.length - failed.length} of ${files.length} test files pass on their own (${seconds}s, ${jobs} at a time)`);
129 process.exit(failed.length === 0 ? 0 : 1);
130 }
scripts/train.test.ts +1 −10
@@ -11,23 +11,14 @@
1111 * hand, and that a retrain writes a file shaped exactly like the one it
1212 * replaces. `src/client/weights.test.ts` is the other half: it holds the file
1313 * that is actually in the tree to the numbers it claims.
14 *
15 * `../src/client/packed.ts` is imported before `./train.ts` on purpose, and
16 * the order is load-bearing until somebody breaks the cycle it works around:
17 * `packed.ts` reads `OBS_SIZE` out of `observe.ts` while it is initialising,
18 * and `observe.ts` → `world.ts` → `bots.ts` → `packed.ts` closes the ring, so
19 * a graph entered at the `observe.ts` end — which is what `./train.ts` is, one
20 * import further on — runs `packed.ts`'s body before that constant exists and
21 * throws `Cannot access 'OBS_SIZE' before initialization`. Entered here, this
22 * file loads on its own as well as in a suite. Not this pull's to fix.
2314 */
2415
2516 import { expect, test } from "bun:test";
2617 import { mkdtempSync, writeFileSync } from "node:fs";
2718 import { tmpdir } from "node:os";
2819 import { join, resolve } from "node:path";
29 import { parsePacked } from "../src/client/packed.ts";
3020 import { cardModule, DEFAULTS, parseOptions, policyDocument, POLICY, USAGE, WEIGHTS, type Options } from "./train.ts";
21 import { parsePacked } from "../src/client/packed.ts";
3122 import { PACKED } from "../src/client/shipped.ts";
3223 import { CARD } from "../src/client/weights.ts";
3324
src/client/grenade.test.ts +3
@@ -32,6 +32,9 @@ import {
3232 type Effect, type Input, type Player, type World,
3333 } from "./world.ts";
3434 import { effectAt, eyeHeight } from "../engine/index.ts";
35 import { useShipped } from "./shipped.ts";
36
37 useShipped(); // rounds here have bots in them, and a bot thinks with the shipped policy
3538
3639 /** Did anything the world remembers look like this? The buffer is read by index. */
3740 function heard(world: World, is: (e: Effect) => boolean): boolean {
src/client/obs-size.ts added +14
@@ -0,0 +1,14 @@
1 /**
2 * How many numbers a bot sees: the length of the vector `observe` fills, and
3 * so the input width of every policy the game ships.
4 *
5 * It is a module of its own that imports nothing so that reading it can never
6 * run into a half-initialised module. `packed.ts` needs it while it is
7 * initialising (`SHAPE` is a constant built from it), and `observe.ts` sits in
8 * a ring — `observe.ts` → `world.ts` → `bots.ts` → `packed.ts` — that, entered
9 * at the `observe.ts` end, used to run `packed.ts`'s body before this constant
10 * existed. `observe.ts` re-exports it, so every caller still imports it from
11 * there; only `packed.ts` reaches here directly.
12 */
13
14 export const OBS_SIZE = 33;
src/client/observe.ts +2 −1
@@ -18,8 +18,9 @@ import { CS_HULL } from "./movement.ts";
1818 import { inaccuracy, WEAPONS } from "./weapons.ts";
1919 import type { Player, World } from "./world.ts";
2020 import { aimAt, bodyCentre, currentWeapon, eyePos } from "./world.ts";
21 import { OBS_SIZE } from "./obs-size.ts";
2122
22 export const OBS_SIZE = 33;
23 export { OBS_SIZE };
2324
2425 /** What the bot remembers between ticks that the world does not store for it. */
2526 export type Memory = {
src/client/packed.ts +1 −1
@@ -22,7 +22,7 @@
2222 */
2323
2424 import { HEADS } from "./action.ts";
25 import { OBS_SIZE } from "./observe.ts";
25 import { OBS_SIZE } from "./obs-size.ts";
2626 import { paramCount, type Packed, type Shape } from "../engine/index.ts";
2727
2828 /**
src/client/world.test.ts +3
@@ -18,6 +18,9 @@ import { CS_HULL } from "./movement.ts";
1818 import {
1919 angleVectors, createEffects, effectAt, eyeHeight, pushEffect, type Burst, type SoundDevice, type Tone, type Vec3,
2020 } from "../engine/index.ts";
21 import { useShipped } from "./shipped.ts";
22
23 useShipped(); // rounds here have bots in them, and a bot thinks with the shipped policy
2124
2225 const TICK = 1 / 64;
2326 const run = (world: World, seconds: number, input: Input = NO_INPUT): void => {
src/games.ts +4
@@ -123,6 +123,10 @@ export const GAMES: readonly Game[] = [
123123 * renderer, the fall in `src/client/scene.ts` and the camera lifted out of
124124 * your own body in `src/client/spectate.ts` — landed under it without a
125125 * raise: 135,163 raw and 52,467 gzipped after the merge, measured the same way.
126 *
127 * #248 moved `OBS_SIZE` into `src/client/obs-size.ts` to break a load-time
128 * cycle, and moved this bundle by the handful the standing fact above says:
129 * 137,564 → 137,568 raw and 53,443 → 53,440 gzipped. No raise.
126130 */
127131 ceiling: { raw: 140_500, gzip: 54_500 },
128132 assets: [

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs