A change to a crate's tests deploys nothing: tests/, benches/, examples/ and #[cfg(test)]-only sources are not what a unit is built from
0cdd221 changed only crates/actions/tests/repository_workflows.rs among Rust files, and the plan sent actions, security and the runner. Each unit now knows the Rust crates it is built from (crateDirs: its own crate, the workspace crates it depends on, and its image's crates), and touches and touchesImage pass over their tests/, benches/ and examples/ folders. A source file under a crate's src/ is passed over too when it is compiled only for tests: every `mod` declaration of it found (in its parent, lib.rs or main.rs at the top, or a #[path] in a file beside it) is under #[cfg(test)], or is in a file that is itself only for tests. The plan reads them at the commit being deployed (git show, git ls-tree), each file once, stopping at the first declaration that is built. A file nothing is found to declare counts as built, so it deploys. Replaying 0cdd221 against its parent now deploys nothing.
4 files+216−40/4 viewed
| 158 | 158 | 1. Plans: for each unit, the live commit; `git diff` from it to `HEAD`; | |
| 159 | 159 | whether the changed files touch the unit (its folder, the crates and | |
| 160 | 160 | packages it is built from, its inputs, lockfile changes that reach it). | |
| 161 | + | A Rust crate's `tests/`, `benches/` and `examples/` are not what it is | |
| 162 | + | built from, and neither is a source file compiled only for tests: one | |
| 163 | + | whose every `mod` declaration is under `#[cfg(test)]` (with or without | |
| 164 | + | `#[path]`), or inside a module that is. A change to a crate's tests | |
| 165 | + | alone deploys nothing (`scripts/deploy/stack.mjs`, `testOnlySource`). | |
| 161 | 166 | 2. Refuses if uncommitted changes touch what would deploy (`--allow-dirty`). | |
| 162 | 167 | 3. Applies every pending migration (`wrangler d1 migrations apply --remote`), | |
| 163 | 168 | in parallel. Any failure stops the deploy before code. |
| 39 | 39 | versionFrom, | |
| 40 | 40 | } from "./cloudflare.mjs"; | |
| 41 | 41 | import { changedNames, parseCargoLock, parseNpmLock, reaches } from "./lockfiles.mjs"; | |
| 42 | − | import { decide, planJson, pool } from "./plan.mjs"; | |
| 42 | + | import { decide, git, planJson, pool } from "./plan.mjs"; | |
| 43 | 43 | import { | |
| 44 | 44 | ROOT, | |
| 45 | 45 | buildGroups, | |
| ⋯ | |||
| 53 | 53 | problems, | |
| 54 | 54 | resolveStack, | |
| 55 | 55 | resolvedStack, | |
| 56 | + | testOnlySource, | |
| 56 | 57 | touches, | |
| 57 | 58 | touchesBase, | |
| 58 | 59 | touchesImage, | |
| ⋯ | |||
| 328 | 329 | assert.ok(!ids(stack.units, ["packages/contracts/src/index.ts"]).includes("events")); | |
| 329 | 330 | }); | |
| 330 | 331 | ||
| 332 | + | test("a crate's tests, benches and examples deploy nothing", () => { | |
| 333 | + | // 0cdd221 changed only this file, and the plan sent three Rust Workers. | |
| 334 | + | assert.deepEqual(ids(stack.units, ["crates/actions/tests/repository_workflows.rs"]), []); | |
| 335 | + | assert.ok(!touchesImage(unit("runner"), ["crates/actions/tests/repository_workflows.rs"])); | |
| 336 | + | assert.deepEqual(ids(stack.units, ["crates/kit/benches/wire.rs", "crates/scan/examples/scan.rs", "services/repos/tests/git.rs"]), []); | |
| 337 | + | // Their sources still do, and a folder merely named like one. | |
| 338 | + | assert.deepEqual(ids(stack.units, ["crates/actions/src/expr.rs"]), ["actions", "security", "runner"]); | |
| 339 | + | assert.ok(touchesImage(unit("runner"), ["crates/actions/src/expr.rs"])); | |
| 340 | + | assert.deepEqual(ids(stack.units, ["services/repos/src/tests/helpers.rs"]), ["repos"]); | |
| 341 | + | assert.deepEqual(ids(stack.units, ["crates/kit/testsuite/a.rs"]), ids(stack.units, ["crates/kit/src/lib.rs"])); | |
| 342 | + | // A unit that is not a crate keeps its whole folder. | |
| 343 | + | assert.deepEqual(unit("web").crateDirs, []); | |
| 344 | + | assert.deepEqual(unit("events").crateDirs, ["crates/contracts", "crates/kit", "services/events"]); | |
| 345 | + | assert.ok(unit("runner").crateDirs.includes("crates/runner")); | |
| 346 | + | }); | |
| 347 | + | ||
| 348 | + | /** Rust sources by path, as `testOnlySource` reads them. */ | |
| 349 | + | const sources = (files) => ({ | |
| 350 | + | read: (path) => files[path] ?? "", | |
| 351 | + | list: (dir) => Object.keys(files).filter((path) => path.slice(0, path.lastIndexOf("/")) === dir), | |
| 352 | + | }); | |
| 353 | + | ||
| 354 | + | test("a Rust file compiled only for tests is found from what declares it", () => { | |
| 355 | + | const crate = ["crates/x"]; | |
| 356 | + | const tree = sources({ | |
| 357 | + | "crates/x/src/lib.rs": "pub mod api;\nmod store;\n#[cfg(test)]\nmod tests;\n#[cfg(test)] pub(crate) mod fixtures;\n#[cfg(any(test, feature = \"x\"))]\nmod both;\n", | |
| 358 | + | "crates/x/src/api.rs": "mod wire;\n#[cfg(test)]\n#[path = \"api_tests.rs\"]\nmod tests;\n", | |
| 359 | + | "crates/x/src/api/wire.rs": "", | |
| 360 | + | "crates/x/src/api_tests.rs": "use super::*;", | |
| 361 | + | "crates/x/src/store/mod.rs": "#[cfg(test)]\nmod memory;\n", | |
| 362 | + | "crates/x/src/store/memory.rs": "mod deep;", | |
| 363 | + | "crates/x/src/store/memory/deep.rs": "", | |
| 364 | + | "crates/x/src/tests.rs": "", | |
| 365 | + | "crates/x/src/fixtures/mod.rs": "", | |
| 366 | + | "crates/x/src/both.rs": "", | |
| 367 | + | "crates/x/src/bin/tool.rs": "", | |
| 368 | + | }); | |
| 369 | + | const only = (file) => testOnlySource(file, crate, tree); | |
| 370 | + | for (const file of ["crates/x/src/tests.rs", "crates/x/src/fixtures/mod.rs", "crates/x/src/api_tests.rs", "crates/x/src/store/memory.rs", "crates/x/src/store/memory/deep.rs"]) { | |
| 371 | + | assert.ok(only(file), file); | |
| 372 | + | } | |
| 373 | + | // Built: roots, ordinary modules, a cfg that is not only test, a file | |
| 374 | + | // nothing declares (new, or deleted), and files outside src or the crate. | |
| 375 | + | for (const file of [ | |
| 376 | + | "crates/x/src/lib.rs", | |
| 377 | + | "crates/x/src/api.rs", | |
| 378 | + | "crates/x/src/api/wire.rs", | |
| 379 | + | "crates/x/src/store/mod.rs", | |
| 380 | + | "crates/x/src/both.rs", | |
| 381 | + | "crates/x/src/bin/tool.rs", | |
| 382 | + | "crates/x/src/new.rs", | |
| 383 | + | "crates/x/build.rs", | |
| 384 | + | "crates/y/src/tests.rs", | |
| 385 | + | "crates/x/src/notes.md", | |
| 386 | + | ]) { | |
| 387 | + | assert.ok(!only(file), file); | |
| 388 | + | } | |
| 389 | + | }); | |
| 390 | + | ||
| 391 | + | test("this repository's test-only sources are read from git", () => { | |
| 392 | + | const head = git.head(); | |
| 393 | + | const atHead = { read: (path) => git.show(head, path), list: (dir) => git.list(head, dir) }; | |
| 394 | + | const only = (u, file) => testOnlySource(file, unit(u).crateDirs, atHead); | |
| 395 | + | assert.ok(only("api", "apps/api/src/responses.rs")); | |
| 396 | + | assert.ok(only("billing", "services/billing/src/catalogue_tests.rs")); | |
| 397 | + | assert.ok(only("billing", "services/billing/src/gateway_tests.rs")); | |
| 398 | + | assert.ok(!only("billing", "services/billing/src/catalogue.rs")); | |
| 399 | + | assert.ok(!only("api", "apps/api/src/toolkit.rs")); | |
| 400 | + | }); | |
| 401 | + | ||
| 402 | + | test("decide: a change only to tests deploys nothing", () => { | |
| 403 | + | const units = ["actions", "billing", "runner"].map(unit); | |
| 404 | + | const live = { actions: { sha: OLD }, billing: { sha: OLD }, runner: { sha: OLD } }; | |
| 405 | + | const files = { | |
| 406 | + | [`${HEAD}:services/billing/src/gateway.rs`]: "#[cfg(test)]\n#[path = \"gateway_tests.rs\"]\nmod tests;\n", | |
| 407 | + | [`${HEAD}:services/billing/src/lib.rs`]: "mod gateway;\n", | |
| 408 | + | }; | |
| 409 | + | const gitApi = { | |
| 410 | + | ...fakeGit(["crates/actions/tests/repository_workflows.rs", "services/billing/src/gateway_tests.rs"]), | |
| 411 | + | show: (sha, path) => files[`${sha}:${path}`] ?? "", | |
| 412 | + | list: (sha, dir) => Object.keys(files).map((k) => k.slice(sha.length + 1)).filter((p) => p.startsWith(`${dir}/`)).concat(dir === "services/billing/src" ? ["services/billing/src/gateway_tests.rs"] : []), | |
| 413 | + | }; | |
| 414 | + | const decisions = decide(units, { live, head: HEAD, gitApi }); | |
| 415 | + | assert.deepEqual(decisions.map((d) => [d.unit.id, d.deploy, d.image]), [["actions", false, false], ["billing", false, false], ["runner", false, false]]); | |
| 416 | + | // With a source that ships beside them, only that is listed. | |
| 417 | + | const mixed = decide([unit("billing")], { live, head: HEAD, gitApi: { ...gitApi, changed: () => ["services/billing/src/gateway_tests.rs", "services/billing/src/gateway.rs"] } }); | |
| 418 | + | assert.deepEqual([mixed[0].deploy, mixed[0].files], [true, ["services/billing/src/gateway.rs"]]); | |
| 419 | + | }); | |
| 420 | + | ||
| 331 | 421 | test("own folders, declared inputs and root files", () => { | |
| 332 | 422 | assert.deepEqual(ids(stack.units, ["services/pages/src/index.ts"]), ["pages"]); | |
| 333 | 423 | assert.deepEqual(ids(stack.units, ["apps/web/app/lib/roadmap.ts"]), ["og", "web"]); | |
| 5 | 5 | import { execFileSync } from "node:child_process"; | |
| 6 | 6 | ||
| 7 | 7 | import { changedNames, lockRoots, parseCargoLock, parseNpmLock, reaches } from "./lockfiles.mjs"; | |
| 8 | − | import { ROOT, byStage, buildGroups, codeStages, touches, touchesImage } from "./stack.mjs"; | |
| 8 | + | import { ROOT, byStage, buildGroups, codeStages, testOnlySource, touches, touchesImage } from "./stack.mjs"; | |
| 9 | 9 | ||
| 10 | 10 | /** Git, read-only. */ | |
| 11 | 11 | export const git = { | |
| ⋯ | |||
| 21 | 21 | return false; | |
| 22 | 22 | } | |
| 23 | 23 | }, | |
| 24 | − | /** Files changed between two commits. */ | |
| 25 | 24 | /** A file's text at a commit, or "" if it is not there. */ | |
| 26 | 25 | show: (sha, path) => { | |
| 27 | 26 | try { | |
| ⋯ | |||
| 30 | 29 | return ""; | |
| 31 | 30 | } | |
| 32 | 31 | }, | |
| 32 | + | /** The files directly in a folder at a commit (repository-relative). */ | |
| 33 | + | list: (sha, dir) => { | |
| 34 | + | try { | |
| 35 | + | return run(["ls-tree", "--name-only", sha, "--", `${dir}/`]).split("\n").filter(Boolean); | |
| 36 | + | } catch { | |
| 37 | + | return []; | |
| 38 | + | } | |
| 39 | + | }, | |
| 40 | + | /** Files changed between two commits. */ | |
| 33 | 41 | changed: (from, to) => run(["diff", "--name-only", "--no-renames", from, to]).split("\n").filter(Boolean), | |
| 34 | 42 | /** Whether `older` is in `newer`'s history (and not the same commit). */ | |
| 35 | 43 | isAncestor: (older, newer) => { | |
| ⋯ | |||
| 82 | 90 | } | |
| 83 | 91 | return locks.get(key); | |
| 84 | 92 | }; | |
| 93 | + | // A Rust source compiled only for tests (`#[cfg(test)] mod tests;`) is | |
| 94 | + | // not in what deploys. Read at the commit being deployed, each file once. | |
| 95 | + | const texts = new Map(); | |
| 96 | + | const listings = new Map(); | |
| 97 | + | const atHead = { | |
| 98 | + | read: (path) => { | |
| 99 | + | if (!texts.has(path)) texts.set(path, gitApi.show(head, path)); | |
| 100 | + | return texts.get(path); | |
| 101 | + | }, | |
| 102 | + | list: (dir) => { | |
| 103 | + | if (!listings.has(dir)) listings.set(dir, gitApi.list?.(head, dir) ?? []); | |
| 104 | + | return listings.get(dir); | |
| 105 | + | }, | |
| 106 | + | }; | |
| 107 | + | const testOnly = new Map(); | |
| 108 | + | const onlyForTests = (unit, file) => { | |
| 109 | + | if (!file.endsWith(".rs") || !unit.crateDirs?.some((dir) => file.startsWith(`${dir}/src/`))) return false; | |
| 110 | + | if (!testOnly.has(file)) testOnly.set(file, testOnlySource(file, unit.crateDirs, atHead)); | |
| 111 | + | return testOnly.get(file); | |
| 112 | + | }; | |
| 85 | 113 | const relevant = (unit, sha, files) => | |
| 86 | 114 | files.filter((file) => { | |
| 115 | + | if (onlyForTests(unit, file)) return false; | |
| 87 | 116 | if (file !== "Cargo.lock" && file !== "package-lock.json") return true; | |
| 88 | 117 | const { after, names } = lockChange(sha, file); | |
| 89 | 118 | const roots = lockRoots(unit)[file === "Cargo.lock" ? "cargo" : "npm"]; | |
| 173 | 173 | unit.crate = crate; | |
| 174 | 174 | unit.pkg = pkg; | |
| 175 | 175 | unit.dependsOn = [...dirs].sort(); | |
| 176 | + | // The Rust crates it is built from, whose tests, benches and examples | |
| 177 | + | // are not. | |
| 178 | + | unit.crateDirs = crate ? [unit.path, ...closure(cargo, crate).map((name) => cargo.get(name).dir)].sort() : []; | |
| 176 | 179 | unit.inputs = [...new Set([...(unit.inputs ?? []), ...globalInputs(unit.kind)])].sort(); | |
| 177 | 180 | if (unit.image) { | |
| 178 | 181 | const name = unit.image.crate; | |
| ⋯ | |||
| 189 | 192 | }; | |
| 190 | 193 | for (const dir of unit.image.dirs) if (dir !== unit.path) unit.dependsOn.push(dir); | |
| 191 | 194 | unit.dependsOn = [...new Set(unit.dependsOn)].sort(); | |
| 195 | + | unit.crateDirs = [...new Set([...unit.crateDirs, ...unit.image.dirs])].sort(); | |
| 192 | 196 | unit.inputs = [...new Set([...unit.inputs, "Cargo.toml", "Cargo.lock", "scripts/build-runner.mjs"])].sort(); | |
| 193 | 197 | } | |
| 194 | 198 | } | |
| ⋯ | |||
| 202 | 206 | ||
| 203 | 207 | const under = (file, dir) => file === dir || file.startsWith(`${dir}/`); | |
| 204 | 208 | ||
| 209 | + | /** A Rust crate's folders that its library and binaries are never built from. */ | |
| 210 | + | export const CRATE_TEST_DIRS = ["tests", "benches", "examples"]; | |
| 211 | + | ||
| 212 | + | /** | |
| 213 | + | * Whether `file` is in the tests, benches or examples of one of | |
| 214 | + | * `crateDirs`: Cargo builds those only for `cargo test`, `cargo bench` and | |
| 215 | + | * `--example`, never into what deploys. | |
| 216 | + | */ | |
| 217 | + | export function inCrateTests(file, crateDirs = []) { | |
| 218 | + | return crateDirs.some((dir) => CRATE_TEST_DIRS.some((sub) => under(file, `${dir}/${sub}`))); | |
| 219 | + | } | |
| 220 | + | ||
| 221 | + | const dirOf = (path) => path.slice(0, Math.max(0, path.lastIndexOf("/"))); | |
| 222 | + | const baseOf = (path) => path.slice(path.lastIndexOf("/") + 1); | |
| 223 | + | ||
| 224 | + | /** | |
| 225 | + | * The `mod name;` declarations in Rust source `text` that `match` (by name, | |
| 226 | + | * or by a `#[path]`), each with whether it is under `#[cfg(test)]`. | |
| 227 | + | * Only outline modules (`mod x;`); the attributes are those directly above. | |
| 228 | + | */ | |
| 229 | + | function declarations(text, match) { | |
| 230 | + | const found = []; | |
| 231 | + | const pattern = /((?:#\[[^\]]*\]\s*)*)(?:pub(?:\([^)]*\))?\s+)?mod\s+(r#)?(\w+)\s*;/g; | |
| 232 | + | for (const [, attrs, , name] of text.matchAll(pattern)) { | |
| 233 | + | const path = /#\[\s*path\s*=\s*"([^"]+)"\s*\]/.exec(attrs)?.[1] ?? null; | |
| 234 | + | if (!match(name, path)) continue; | |
| 235 | + | found.push(/#\[\s*cfg\s*\(\s*test\s*\)\s*\]/.test(attrs)); | |
| 236 | + | } | |
| 237 | + | return found; | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | /** | |
| 241 | + | * Whether Rust source `file`, in the `src/` of one of `crateDirs`, is | |
| 242 | + | * compiled only for tests: every module declaration of it found is under | |
| 243 | + | * `#[cfg(test)]`, or is in a file that is itself only for tests. A file | |
| 244 | + | * nothing is found to declare (a new crate root, a deleted file, a module | |
| 245 | + | * declared some way this does not read) counts as built, so a change to it | |
| 246 | + | * deploys. | |
| 247 | + | * | |
| 248 | + | * read(path): a file's text at the commit being deployed, "" if absent | |
| 249 | + | * list(dir): the files directly in a folder at that commit | |
| 250 | + | */ | |
| 251 | + | export function testOnlySource(file, crateDirs, { read, list = () => [] }, depth = 0) { | |
| 252 | + | if (!file.endsWith(".rs") || depth > 16) return false; | |
| 253 | + | const crateDir = crateDirs.find((dir) => file.startsWith(`${dir}/src/`)); | |
| 254 | + | if (!crateDir) return false; | |
| 255 | + | const src = `${crateDir}/src`; | |
| 256 | + | const dir = dirOf(file); | |
| 257 | + | const base = baseOf(file).slice(0, -".rs".length); | |
| 258 | + | // Crate roots and binaries are not declared by anything. | |
| 259 | + | if (dir === src && ["lib", "main"].includes(base)) return false; | |
| 260 | + | if (dir === `${src}/bin` || (base === "main" && dirOf(dir) === `${src}/bin`)) return false; | |
| 261 | + | // Where `mod name;` for this file would be: `a/name.rs` and `a/name/mod.rs` | |
| 262 | + | // are declared by `a.rs` or `a/mod.rs` (by `lib.rs` or `main.rs` in src). | |
| 263 | + | const name = base === "mod" ? baseOf(dir) : base; | |
| 264 | + | const parentDir = base === "mod" ? dirOf(dir) : dir; | |
| 265 | + | const parents = | |
| 266 | + | parentDir === src ? [`${src}/lib.rs`, `${src}/main.rs`] : [`${parentDir}/mod.rs`, `${dirOf(parentDir)}/${baseOf(parentDir)}.rs`]; | |
| 267 | + | // One declaration that is built is enough to stop: most changed files | |
| 268 | + | // are ordinary modules, settled by reading their parent. | |
| 269 | + | let declared = false; | |
| 270 | + | const testOnly = (declarer, isTest) => { | |
| 271 | + | declared = true; | |
| 272 | + | return isTest || testOnlySource(declarer, crateDirs, { read, list }, depth + 1); | |
| 273 | + | }; | |
| 274 | + | for (const parent of parents) { | |
| 275 | + | for (const isTest of declarations(read(parent), (found, path) => found === name && path === null)) { | |
| 276 | + | if (!testOnly(parent, isTest)) return false; | |
| 277 | + | } | |
| 278 | + | } | |
| 279 | + | // `#[path = "x.rs"] mod y;` in a file beside it names it directly. | |
| 280 | + | const fileName = baseOf(file); | |
| 281 | + | for (const sibling of list(dir).filter((path) => path.endsWith(".rs") && path !== file)) { | |
| 282 | + | for (const isTest of declarations(read(sibling), (_, path) => path === fileName || path === `./${fileName}`)) { | |
| 283 | + | if (!testOnly(sibling, isTest)) return false; | |
| 284 | + | } | |
| 285 | + | } | |
| 286 | + | return declared; | |
| 287 | + | } | |
| 288 | + | ||
| 205 | 289 | /** | |
| 206 | 290 | * Why a set of changed files (repository-relative, `/`-separated) touches | |
| 207 | 291 | * a unit: the first file that does, and through what. Null if none does. | |
| 292 | + | * Its crates' tests, benches and examples do not. | |
| 208 | 293 | */ | |
| 209 | 294 | export function touches(unit, files) { | |
| 295 | + | files = files.filter((file) => !inCrateTests(file, unit.crateDirs)); | |
| 210 | 296 | for (const file of files) { | |
| 211 | 297 | if (under(file, unit.path)) return { file, via: "its own folder" }; | |
| 212 | 298 | } | |
| ⋯ | |||
| 221 | 307 | /** Whether changed files touch a unit's Containers image. */ | |
| 222 | 308 | export function touchesImage(unit, files) { | |
| 223 | 309 | if (!unit.image) return false; | |
| 224 | − | return files.some((file) => unit.image.files.includes(file) || unit.image.dirs.some((dir) => under(file, dir))); | |
| 310 | + | return files.some( | |
| 311 | + | (file) => unit.image.files.includes(file) || (unit.image.dirs.some((dir) => under(file, dir)) && !inCrateTests(file, unit.image.dirs)), | |
| 312 | + | ); | |
| 225 | 313 | } | |
| 226 | 314 | ||
| 227 | 315 | /** Whether changed files touch the folder a unit's base image is built from. */ | |