Artifact b034b33b19dfffd971666f6b53096747f2878ed11ca2d15b05fcc8b7e0c8b78a:
- File
src/tidy.zig
— part of check-in
[9033c13f21]
at
2026-09-17 17:04:25
on branch trunk
— Remove issue-tracker numbers from source and documents
The tracker is disposable and does not live with the source, so a number is a dangling pointer. Every zit#NN and #NN reference in code comments, test names, help text, smoke labels, identifiers, release notes and errata is replaced by enough context to stand alone, or by a short plain statement of the problem. No behaviour changed. (user: dmestas, size: 13964) [annotate] [blame] [check-ins using]
//! tidy: a dead-file guard over the src/ test surface. //! //! Zig compiles and runs only the tests reachable from a root the build wires. //! A `.zig` file that no root imports still COMPILES — its tests simply stop //! running, and the coverage evaporates with no error. This guard closes that //! hazard: it walks `src/**/*.zig`, builds the `@import` graph, and asserts every //! file is reachable from a root `zig build test` runs — the library, CLI, and //! DST roots plus the separately compiled Git-import test roots. Keeping those //! importer tests as build roots avoids path-importing their private modules //! into the public `zit` module; dedicated process and benchmark roots are //! instead reviewed in `allowlist`. //! //! The mechanism is borrowed from TigerBeetle's `src/tidy.zig` DeadFilesDetector //! (github.com/tigerbeetle/tigerbeetle), which walks its own source tree and //! fails the build when a file falls out of the import graph. //! //! It exists BEFORE the `main.zig` -> `src/cmd/` split precisely so that split //! cannot silently orphan a file's tests: move a file, forget to re-root it, and //! this test names the file and the one-line fix. //! //! Imports are extracted with the Zig tokenizer, not a byte scan, so an //! `@import("...")` written inside a string literal or comment — as this very //! file does in the remediation message below — is correctly NOT read as an //! import edge. const std = @import("std"); /// The repository root, wired in build.zig's `build_unit_test_module` (the same /// handle src/packaging_test.zig uses). A test binary's working directory is not /// the checkout, so the src tree is opened by this absolute path rather than a /// cwd-relative one. const repo_options = @import("repo_options"); /// The test roots `zig build test` executes, each of which aggregates other /// files through `_ = @import("...")` lines that can be dropped by hand, plus /// the independent importer test modules build.zig runs through the same test /// step. Paths are relative to `src/`. const roots = [_][]const u8{ "root.zig", "main.zig", "dst.zig", "git/git_import_test.zig", "git/git_import_refs_test.zig", "git/git_import_cli_test.zig", "git/git_export_cli_test.zig", "git/git_export_e2e.zig", }; /// Files legitimately unreachable from the test roots because build.zig wires /// them as their OWN roots through dedicated steps. Each was verified against /// build.zig — this is the reviewed exception set, not a convenience to silence /// a real orphan. If one of these ever becomes reachable from a test root, the /// hygiene check below fails and demands the entry be removed. Paths are /// relative to `src/`. const allowlist = [_][]const u8{ // Root of `zig build e2e`. `zig build test` compiles it as a type-check gate // but never runs it (it spawns the real `zit` binary ~150x). Its own tests // run under the e2e step, so it needs no test-root import. "e2e.zig", // Root of `zig build smoke`. Compiled-but-not-run by `test`, run only by the // milestone-manual smoke step that drives the real `fossil` binary. "smoke.zig", // Root of `zig build muscle`: the fossil-muscle-memory // differential transcript. Same shape as smoke — compiled by `test` // (its judge has unit tests there), run only by the manual step. "muscle.zig", // Root of `zig build bench`. build.zig ALSO compiles it into a test binary // (`bench_tests`) that joins `zig build test`, so its inline tests do run — // just not through a tidy root. It is its own root. "logstore_benchmark.zig", // Reached only from logstore_benchmark.zig (the bench root above); their // tests ride the same `bench_tests` binary into `zig build test`. "storage_benchmark.zig", "sync_benchmark.zig", "warmread_benchmark.zig", // The `serve` readiness probe (`zig build bench -- serve`), dispatched from // logstore_benchmark.zig; its pure tests ride the same `bench_tests` binary. "serve_benchmark.zig", // `git-import-e2e` is compile-gated by `test` but runs only by its explicit // process-heavy step, just like the general e2e root above. "git/git_import_e2e.zig", // The importer benchmark is out-of-band, and this support module is reached // only from that separate executable root. "git/git_import_benchmark.zig", // Build-only pipe helper used by the CLI transaction test's build option. // It is an executable root, not a module a test root can import. "git/git_import_cli_pipe_helper.zig", }; /// Vacuity floor: the walk must find at least this many `.zig` files or the tree /// was not actually scanned (wrong path, empty handle) and every check below /// would pass by finding nothing. The tree holds ~60 files today; 40 is a floor /// with headroom, not a running count to keep in sync. const min_expected_files = 40; /// Read cap per source file. The largest src file is well under 1 MiB; 8 MiB is /// a tripwire against a runaway read, not an expected size. const max_source_bytes = 8 << 20; test "every src/**/*.zig is reachable from a test root" { const io = std.testing.io; // One arena for every path string and graph structure: keys stay pointer // stable for the hash maps, and a single deinit reclaims all of it. var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena_state.deinit(); const arena = arena_state.allocator(); const src_abs = try std.fmt.allocPrint(arena, "{s}/src", .{repo_options.root}); var src_dir = std.Io.Dir.cwd().openDir(io, src_abs, .{ .iterate = true }) catch |err| switch (err) { // The only legitimate skip: the wired root is not a directory here, so // the machine cannot answer the question — the same stance the // version-control pin in packaging_test.zig takes. Anything else is a // real fault and must fail loudly rather than skip into silence. error.FileNotFound, error.NotDir => return error.SkipZigTest, else => { std.debug.print("\ntidy: cannot open {s}: {s}\n", .{ src_abs, @errorName(err) }); return error.TidyWalkFailed; }, }; defer src_dir.close(io); // Pass 1 — every src/**/*.zig, path relative to src/. On POSIX the walker's // separator is '/', matching both the import strings and the roots/allowlist // above; a '\\'-separated platform would make every lookup miss and fail this // guard loudly, never pass it wrongly. var files: std.ArrayList([]const u8) = .empty; var file_set: std.StringHashMapUnmanaged(void) = .empty; { var walker = try src_dir.walk(arena); defer walker.deinit(); while (try walker.next(io)) |entry| { if (entry.kind != .file) continue; if (!std.mem.endsWith(u8, entry.path, ".zig")) continue; const rel = try arena.dupe(u8, entry.path); try files.append(arena, rel); try file_set.put(arena, rel, {}); } } // Vacuity guard: a silent zero (wrong cwd, empty handle) would let every // assertion below pass by finding nothing to check. if (files.items.len < min_expected_files) { std.debug.print("\ntidy: walked {s} and found only {d} .zig files; expected at least {d}. " ++ "The walk is not seeing the source tree — this guard would pass vacuously.\n", .{ src_abs, files.items.len, min_expected_files }); return error.TidyWalkFailed; } // Each root must be a real file, or reachability starts from nothing and // waves everything through. A renamed or moved root has to fail here. for (roots) |root| { if (!file_set.contains(root)) { std.debug.print("\ntidy: test root \"{s}\" does not exist under src/. The reachability " ++ "guard starts from the roots build.zig wires into `zig build test`; update this list when a root moves.\n", .{root}); return error.TidyRootMissing; } } // Pass 2 — breadth-first over the @import graph from every test root. var reached: std.StringHashMapUnmanaged(void) = .empty; var queue: std.ArrayList([]const u8) = .empty; for (roots) |root| { const key = file_set.getKey(root).?; // just asserted present if ((try reached.getOrPut(arena, key)).found_existing) continue; try queue.append(arena, key); } var head: usize = 0; while (head < queue.items.len) : (head += 1) { const file = queue.items[head]; const source = try src_dir.readFileAllocOptions(io, file, arena, .limited(max_source_bytes), .of(u8), 0); var imports: std.ArrayList([]const u8) = .empty; try collectImports(arena, source, parentDir(file), &imports); for (imports.items) |target| { // Imports of `std`, `zit`, `build_options`, `repo_options`, etc. carry // no `.zig` and are already filtered; a resolved path not in the file // set is a cross-module or generated import and is not our concern. const key = file_set.getKey(target) orelse continue; if ((try reached.getOrPut(arena, key)).found_existing) continue; try queue.append(arena, key); } } // Allowlist hygiene: every entry must be a real file that is genuinely // unreachable. A stale entry (file gone) or a dead one (now reachable) means // the list papers over nothing — fail so it cannot rot into cover for a // future real orphan. for (allowlist) |allowed| { if (!file_set.contains(allowed)) { std.debug.print("\ntidy: allowlist names \"{s}\", which no longer exists under src/. " ++ "Remove the stale entry.\n", .{allowed}); return error.TidyAllowlistStale; } if (reached.contains(allowed)) { std.debug.print("\ntidy: allowlist names \"{s}\", but it is now reachable from a test root. " ++ "Remove it from the allowlist — the exemption is dead.\n", .{allowed}); return error.TidyAllowlistDead; } } // The assertion: every src file is reached or explicitly allowlisted. var orphans: usize = 0; for (files.items) |file| { if (reached.contains(file)) continue; if (isAllowlisted(file)) continue; orphans += 1; std.debug.print( \\ \\tidy: src/{s} is not reachable from any test root. \\ Add `_ = @import("{s}");` to the test root that owns its module, or \\ add a reviewed build-only exception when an import would duplicate a module. \\ A file no test root imports still compiles — its tests just never run. \\ , .{ file, file }); } if (orphans != 0) return error.TidyDeadFile; } fn isAllowlisted(file: []const u8) bool { for (allowlist) |a| { if (std.mem.eql(u8, a, file)) return true; } return false; } /// The directory portion of a src-relative path: `sim/net.zig` -> `sim`, and a /// file directly under src/ -> `""`. fn parentDir(path: []const u8) []const u8 { const slash = std.mem.lastIndexOfScalar(u8, path, '/') orelse return ""; return path[0..slash]; } /// Extract every `@import("...zig")` from `source`, resolve each against the /// importing file's directory, and append the src-relative results to `out`. /// /// The Zig tokenizer drives this so imports inside comments or string literals /// are not mistaken for edges. Import path literals carry no escapes, so the slice /// between the string_literal token's quotes is the literal path. fn collectImports( arena: std.mem.Allocator, source: [:0]const u8, importer_dir: []const u8, out: *std.ArrayList([]const u8), ) !void { var tokenizer = std.zig.Tokenizer.init(source); while (true) { const builtin = tokenizer.next(); if (builtin.tag == .eof) break; if (builtin.tag != .builtin) continue; if (!std.mem.eql(u8, source[builtin.loc.start..builtin.loc.end], "@import")) continue; const lparen = tokenizer.next(); if (lparen.tag != .l_paren) continue; const literal = tokenizer.next(); if (literal.tag != .string_literal) continue; std.debug.assert(literal.loc.end - literal.loc.start >= 2); const raw = source[literal.loc.start + 1 .. literal.loc.end - 1]; if (!std.mem.endsWith(u8, raw, ".zig")) continue; try out.append(arena, try resolveImport(arena, importer_dir, raw)); } } /// Join an import target onto the importing file's directory and collapse `.` /// and `..` components, yielding an arena-owned src-relative path. No real import /// escapes src/ today; the `..` handling keeps the resolver correct if one ever /// does. fn resolveImport(arena: std.mem.Allocator, dir: []const u8, target: []const u8) ![]const u8 { var components: std.ArrayList([]const u8) = .empty; defer components.deinit(arena); var dir_it = std.mem.splitScalar(u8, dir, '/'); while (dir_it.next()) |component| try pushComponent(&components, arena, component); var target_it = std.mem.splitScalar(u8, target, '/'); while (target_it.next()) |component| try pushComponent(&components, arena, component); var out: std.ArrayList(u8) = .empty; defer out.deinit(arena); for (components.items, 0..) |component, i| { if (i != 0) try out.append(arena, '/'); try out.appendSlice(arena, component); } return arena.dupe(u8, out.items); } fn pushComponent(components: *std.ArrayList([]const u8), arena: std.mem.Allocator, component: []const u8) !void { if (component.len == 0 or std.mem.eql(u8, component, ".")) return; if (std.mem.eql(u8, component, "..")) { if (components.items.len != 0) _ = components.pop(); return; } try components.append(arena, component); }