ReviewOS

also looking at this

zig-utils/zig-test-framework

Add deterministic test sharding for distributed CI

#22
Merged glennmichael123 wants to merge codex/issue-4-deterministic-sharding into main
10 files +322 -5

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

build.zigmodified+10-0
Changes to build.zig
@@ -276,6 +276,16 @@ pub fn build(b: *std.Build) void {
276276 run_invalid_config.expectExitCode(2);
277277 test_step.dependOn(&run_invalid_config.step);
278278
279 // Exercise both sides of a file-level shard split. One shard runs the
280 // fixture and the other is intentionally empty; both are valid CI jobs.
281 const run_shard_one = b.addRunArtifact(exe);
282 run_shard_one.addArgs(&.{ "--test-dir", "tests", "--pattern", "sample.test.zig", "--shard-index", "1", "--shard-count", "2", "--no-color" });
283 test_step.dependOn(&run_shard_one.step);
284
285 const run_shard_two = b.addRunArtifact(exe);
286 run_shard_two.addArgs(&.{ "--test-dir", "tests", "--pattern", "sample.test.zig", "--shard-index", "2", "--shard-count", "2", "--no-color" });
287 test_step.dependOn(&run_shard_two.step);
288
279289 const run_version = b.addRunArtifact(exe);
280290 run_version.addArg("--version");
281291 run_version.expectStdErrEqual(b.fmt("Zig Test Framework v{s}\n", .{package_version}));
src/cli.zigmodified+65-0
Changes to src/cli.zig
@@ -16,6 +16,8 @@ pub const CLIOptions = struct {
1616 test_dir: ?[]const u8 = ".",
1717 pattern: []const u8 = "*.test.zig",
1818 no_recursive: bool = false,
19 shard_index: ?usize = null,
20 shard_count: ?usize = null,
1921 // Coverage options
2022 coverage: bool = false,
2123 coverage_dir: []const u8 = "coverage",
@@ -73,6 +75,8 @@ pub const CLI = struct {
7375 self.options.no_recursive = !config.test_options.recursive;
7476 self.options.filter = config.test_options.filter;
7577 self.options.timeout = config.test_options.timeout;
78 self.options.shard_index = config.sharding.index;
79 self.options.shard_count = config.sharding.count;
7680 self.options.parallel = config.parallel.enabled;
7781 self.options.jobs = config.parallel.jobs;
7882 self.options.reporter = try reporterFromName(config.reporter.reporter);
@@ -144,6 +148,26 @@ pub const CLI = struct {
144148 self.options.pattern = args[i];
145149 } else if (std.mem.eql(u8, arg, "--no-recursive")) {
146150 self.options.no_recursive = true;
151 } else if (std.mem.eql(u8, arg, "--shard-index")) {
152 if (i + 1 >= args.len) {
153 std.debug.print("Error: --shard-index requires a value\n", .{});
154 return CLIError.MissingValue;
155 }
156 i += 1;
157 self.options.shard_index = std.fmt.parseInt(usize, args[i], 10) catch {
158 std.debug.print("Error: --shard-index must be a valid number\n", .{});
159 return CLIError.InvalidArgument;
160 };
161 } else if (std.mem.eql(u8, arg, "--shard-count")) {
162 if (i + 1 >= args.len) {
163 std.debug.print("Error: --shard-count requires a value\n", .{});
164 return CLIError.MissingValue;
165 }
166 i += 1;
167 self.options.shard_count = std.fmt.parseInt(usize, args[i], 10) catch {
168 std.debug.print("Error: --shard-count must be a valid number\n", .{});
169 return CLIError.InvalidArgument;
170 };
147171 } else if (std.mem.eql(u8, arg, "--coverage")) {
148172 self.options.coverage = true;
149173 } else if (std.mem.eql(u8, arg, "--coverage-dir")) {
@@ -273,6 +297,24 @@ pub const CLI = struct {
273297 self.options.test_dir = arg;
274298 }
275299 }
300
301 const has_shard_index = self.options.shard_index != null;
302 const has_shard_count = self.options.shard_count != null;
303 if (has_shard_index != has_shard_count) {
304 std.debug.print("Error: --shard-index and --shard-count must be provided together\n", .{});
305 return CLIError.InvalidArgument;
306 }
307 if (self.options.shard_count) |count| {
308 const index = self.options.shard_index.?;
309 if (count == 0) {
310 std.debug.print("Error: --shard-count must be greater than zero\n", .{});
311 return CLIError.InvalidArgument;
312 }
313 if (index == 0 or index > count) {
314 std.debug.print("Error: --shard-index must be between 1 and --shard-count\n", .{});
315 return CLIError.InvalidArgument;
316 }
317 }
276318 }
277319
278320 /// Return the first option that discovery mode cannot currently honor.
@@ -325,6 +367,8 @@ pub const CLI = struct {
325367 \\ --test-dir <dir> Directory to search for tests (default: .)
326368 \\ --pattern <pattern> Test file pattern (default: *.test.zig)
327369 \\ --no-recursive Disable recursive directory search
370 \\ --shard-index <N> One-based shard to run
371 \\ --shard-count <N> Total number of shards
328372 \\
329373 \\COVERAGE:
330374 \\ --coverage Enable code coverage collection
@@ -458,6 +502,27 @@ test "CLI parse coverage flag" {
458502 try std.testing.expect(cli.options.coverage);
459503}
460504
505test "CLI parse shard options" {
506 var cli = CLI.init(std.testing.allocator);
507 try cli.parse(&.{ "zig-test", "--shard-count", "5", "--shard-index", "3" });
508 try std.testing.expectEqual(@as(?usize, 3), cli.options.shard_index);
509 try std.testing.expectEqual(@as(?usize, 5), cli.options.shard_count);
510}
511
512test "CLI rejects incomplete and out-of-range shards" {
513 var incomplete = CLI.init(std.testing.allocator);
514 try std.testing.expectError(
515 CLIError.InvalidArgument,
516 incomplete.parse(&.{ "zig-test", "--shard-count", "2" }),
517 );
518
519 var out_of_range = CLI.init(std.testing.allocator);
520 try std.testing.expectError(
521 CLIError.InvalidArgument,
522 out_of_range.parse(&.{ "zig-test", "--shard-index", "0", "--shard-count", "2" }),
523 );
524}
525
461526test "CLI parse coverage-dir" {
462527 var cli = CLI.init(std.testing.allocator);
463528 const args = [_][]const u8{ "zig-test", "--coverage-dir", "my-coverage" };
src/config.zigmodified+25-0
Changes to src/config.zig
@@ -8,6 +8,7 @@ pub const ConfigFormat = enum { json };
88/// actually honor. Defaults intentionally match `CLIOptions`.
99pub const TestConfig = struct {
1010 test_options: TestOptions = .{},
11 sharding: ShardingOptions = .{},
1112 parallel: ParallelOptions = .{},
1213 reporter: ReporterOptions = .{},
1314 snapshot: SnapshotOptions = .{},
@@ -17,6 +18,11 @@ pub const TestConfig = struct {
1718 coverage: CoverageOptions = .{},
1819};
1920
21pub const ShardingOptions = struct {
22 index: ?usize = null,
23 count: ?usize = null,
24};
25
2026pub const TestOptions = struct {
2127 pattern: []const u8 = "*.test.zig",
2228 test_dir: []const u8 = ".",
@@ -64,6 +70,7 @@ pub const CoverageOptions = struct {
6470
6571const FileConfig = struct {
6672 @"test": TestOptions = .{},
73 sharding: ShardingOptions = .{},
6774 parallel: ParallelOptions = .{},
6875 reporter: ReporterOptions = .{},
6976 snapshot: SnapshotOptions = .{},
@@ -80,6 +87,7 @@ pub const ConfigError = error{
8087 InvalidPort,
8188 InvalidTimeout,
8289 InvalidDebounce,
90 InvalidShard,
8391};
8492
8593/// Owns strings parsed from configuration files until `deinit` is called.
@@ -116,6 +124,7 @@ pub const ConfigLoader = struct {
116124 );
117125 const config = TestConfig{
118126 .test_options = file_config.@"test",
127 .sharding = file_config.sharding,
119128 .parallel = file_config.parallel,
120129 .reporter = file_config.reporter,
121130 .snapshot = file_config.snapshot,
@@ -175,6 +184,13 @@ fn validate(config: TestConfig) !void {
175184 if (timeout == 0) return ConfigError.InvalidTimeout;
176185 }
177186 if (config.watch.debounce_ms == 0) return ConfigError.InvalidDebounce;
187 const has_shard_index = config.sharding.index != null;
188 const has_shard_count = config.sharding.count != null;
189 if (has_shard_index != has_shard_count) return ConfigError.InvalidShard;
190 if (config.sharding.count) |count| {
191 const index = config.sharding.index.?;
192 if (count == 0 or index == 0 or index > count) return ConfigError.InvalidShard;
193 }
178194}
179195
180196test "TestConfig defaults match CLI discovery defaults" {
@@ -183,6 +199,8 @@ test "TestConfig defaults match CLI discovery defaults" {
183199 try std.testing.expectEqualStrings(".", config.test_options.test_dir);
184200 try std.testing.expect(config.test_options.recursive);
185201 try std.testing.expect(config.test_options.timeout == null);
202 try std.testing.expect(config.sharding.index == null);
203 try std.testing.expect(config.sharding.count == null);
186204 try std.testing.expect(!config.parallel.enabled);
187205 try std.testing.expectEqualStrings("spec", config.reporter.reporter);
188206}
@@ -200,6 +218,11 @@ test "configuration validation rejects invalid runtime values" {
200218 invalid.reporter.reporter = "spec";
201219 invalid.parallel.jobs = 0;
202220 try std.testing.expectError(ConfigError.InvalidJobs, validate(invalid));
221 invalid.parallel.jobs = null;
222 invalid.sharding.index = 2;
223 try std.testing.expectError(ConfigError.InvalidShard, validate(invalid));
224 invalid.sharding.count = 1;
225 try std.testing.expectError(ConfigError.InvalidShard, validate(invalid));
203226}
204227
205228test "ConfigLoader loads supported JSON and rejects unknown keys" {
@@ -210,6 +233,8 @@ test "ConfigLoader loads supported JSON and rejects unknown keys" {
210233 try std.testing.expectEqualStrings("tests/fixtures", config.test_options.test_dir);
211234 try std.testing.expectEqualStrings("selected test passes", config.test_options.filter.?);
212235 try std.testing.expect(!config.test_options.recursive);
236 try std.testing.expectEqual(@as(?usize, 1), config.sharding.index);
237 try std.testing.expectEqual(@as(?usize, 1), config.sharding.count);
213238
214239 try std.testing.expectError(
215240 error.UnknownField,
src/lib.zigmodified+6-0
Changes to src/lib.zig
@@ -9,6 +9,7 @@ pub const matchers = ;
99pub const mock = ;
1010pub const cli = ;
1111pub const discovery = ;
12pub const sharding = ;
1213pub const test_loader = ;
1314pub const coverage = ;
1415pub const parallel = ;
@@ -106,6 +107,11 @@ pub const DiscoveryResult = discovery.DiscoveryResult;
106107pub const TestFile = discovery.TestFile;
107108pub const discoverTests = discovery.discoverTests;
108109
110// Test Sharding
111pub const ShardOptions = sharding.ShardOptions;
112pub const ShardError = sharding.ShardError;
113pub const shardIndex = sharding.shardIndex;
114
109115// Test Loader
110116pub const LoaderOptions = test_loader.LoaderOptions;
111117pub const runDiscoveredTests = test_loader.runDiscoveredTests;
src/main.zigmodified+6-0
Changes to src/main.zig
@@ -161,6 +161,10 @@ pub fn main(init: std.process.Init.Minimal) !void {
161161 }
162162
163163 var all_passed: bool = undefined;
164 const shard_options: ?lib.ShardOptions = if (cli_parser.options.shard_index) |index|
165 .{ .index = index, .count = cli_parser.options.shard_count.? }
166 else
167 null;
164168
165169 // Check if watch mode is enabled
166170 if (cli_parser.options.watch) {
@@ -201,6 +205,7 @@ pub fn main(init: std.process.Init.Minimal) !void {
201205 .filter = cli_parser.options.filter,
202206 .verbose = cli_parser.options.verbose,
203207 .use_colors = !cli_parser.options.no_color,
208 .shard = shard_options,
204209 .coverage_options = cov_opts,
205210 .ui_server = if (ui_server) |*server| server else null,
206211 };
@@ -239,6 +244,7 @@ pub fn main(init: std.process.Init.Minimal) !void {
239244 .filter = cli_parser.options.filter,
240245 .verbose = cli_parser.options.verbose,
241246 .use_colors = !cli_parser.options.no_color,
247 .shard = shard_options,
242248 .coverage_options = cov_opts,
243249 .ui_server = if (ui_server) |*server| server else null,
244250 };
src/sharding.zigadded+87-0
Changes to src/sharding.zig
@@ -0,0 +1,87 @@
1const std = ;
2
3pub const ShardError = error{
4 InvalidShardCount,
5 InvalidShardIndex,
6};
7
8/// A one-based shard selection suitable for CI matrix values.
9pub const ShardOptions = struct {
10 index: usize,
11 count: usize,
12
13 pub fn validate(self: ShardOptions) ShardError!void {
14 if (self.count == 0) return ShardError.InvalidShardCount;
15 if (self.index == 0 or self.index > self.count) return ShardError.InvalidShardIndex;
16 }
17
18 pub fn includes(self: ShardOptions, identity: []const u8) ShardError!bool {
19 try self.validate();
20 return (try shardIndex(identity, self.count)) + 1 == self.index;
21 }
22};
23
24/// Assign a normalized test identity to a zero-based shard using FNV-1a 64.
25/// Path separators are normalized so the same relative path maps identically
26/// on Unix and Windows. The algorithm and seed are intentionally fixed.
27pub fn shardIndex(identity: []const u8, shard_count: usize) ShardError!usize {
28 if (shard_count == 0) return ShardError.InvalidShardCount;
29
30 var hash: u64 = 14_695_981_039_346_656_037;
31 for (identity) |byte| {
32 const normalized = if (byte == '\\') @as(u8, '/') else byte;
33 hash ^= normalized;
34 hash *%= 1_099_511_628_211;
35 }
36
37 const count: u64 = @intCast(shard_count);
38 return @intCast(hash % count);
39}
40
41test "fixed identities have stable shard assignments" {
42 try std.testing.expectEqual(@as(usize, 1), try shardIndex("tests/api.test.zig", 4));
43 try std.testing.expectEqual(@as(usize, 1), try shardIndex("tests/db.test.zig", 4));
44 try std.testing.expectEqual(@as(usize, 2), try shardIndex("tests/unit/math.test.zig", 4));
45}
46
47test "path separators do not change shard assignment" {
48 try std.testing.expectEqual(
49 try shardIndex("tests/unit/math.test.zig", 7),
50 try shardIndex("tests\\unit\\math.test.zig", 7),
51 );
52}
53
54test "every identity belongs to exactly one shard" {
55 const identities = [_][]const u8{
56 "tests/api.test.zig",
57 "tests/db.test.zig",
58 "tests/unit/math.test.zig",
59 "tests/unit/string.test.zig",
60 };
61
62 for (identities) |identity| {
63 var matches: usize = 0;
64 for (1..5) |index| {
65 if (try (ShardOptions{ .index = index, .count = 4 }).includes(identity)) {
66 matches += 1;
67 }
68 }
69 try std.testing.expectEqual(@as(usize, 1), matches);
70 }
71}
72
73test "invalid shard options are rejected" {
74 try std.testing.expectError(
75 ShardError.InvalidShardCount,
76 (ShardOptions{ .index = 1, .count = 0 }).validate(),
77 );
78 try std.testing.expectError(ShardError.InvalidShardCount, shardIndex("test.zig", 0));
79 try std.testing.expectError(
80 ShardError.InvalidShardIndex,
81 (ShardOptions{ .index = 0, .count = 2 }).validate(),
82 );
83 try std.testing.expectError(
84 ShardError.InvalidShardIndex,
85 (ShardOptions{ .index = 3, .count = 2 }).validate(),
86 );
87}
src/test_loader.zigmodified+58-3
Changes to src/test_loader.zig
@@ -1,5 +1,6 @@
11const std = ;
22const discovery = ;
3const sharding = ;
34const coverage = ;
45const ui_server = ;
56const compat = ;
@@ -14,6 +15,8 @@ pub const LoaderOptions = struct {
1415 verbose: bool = false,
1516 /// Whether child Zig test processes may emit colors
1617 use_colors: bool = true,
18 /// Optional one-based file shard to execute
19 shard: ?sharding.ShardOptions = null,
1720 /// Coverage options
1821 coverage_options: ?coverage.CoverageOptions = null,
1922 /// UI server for real-time updates
@@ -31,8 +34,17 @@ pub fn runDiscoveredTests(
3134 return false;
3235 }
3336
34 std.debug.print("Found {} test file(s):\n", .{discovered.files.items.len});
37 const selected_files = try selectedFileCount(discovered, options.shard);
38 if (options.shard) |shard| {
39 std.debug.print(
40 "Shard {}/{}: selected {} of {} test file(s).\n",
41 .{ shard.index, shard.count, selected_files, discovered.files.items.len },
42 );
43 }
44
45 std.debug.print("Found {} test file(s):\n", .{selected_files});
3546 for (discovered.files.items) |file| {
47 if (!try fileSelected(file.relative_path, options.shard)) continue;
3648 std.debug.print(" - {s}\n", .{file.relative_path});
3749 }
3850 std.debug.print("\n", .{});
@@ -40,7 +52,7 @@ pub fn runDiscoveredTests(
4052 // Notify UI of run start
4153 if (options.ui_server) |server| {
4254 var buffer: [256]u8 = undefined;
43 const json = try std.fmt.bufPrint(&buffer, "", .{discovered.files.items.len});
55 const json = try std.fmt.bufPrint(&buffer, "", .{selected_files});
4456 try server.broadcast("run_start", json);
4557 }
4658
@@ -57,6 +69,8 @@ pub fn runDiscoveredTests(
5769 }
5870
5971 for (discovered.files.items) |file| {
72 if (!try fileSelected(file.relative_path, options.shard)) continue;
73
6074 // Run each test file using zig test command
6175 std.debug.print("Running {s}...\n", .{file.relative_path});
6276
@@ -102,7 +116,7 @@ pub fn runDiscoveredTests(
102116
103117 // Print coverage summary if enabled
104118 if (options.coverage_options) |cov_opts| {
105 if (cov_opts.enabled) {
119 if (cov_opts.enabled and files_run > 0) {
106120 std.debug.print("\n", .{});
107121 const cov_result = coverage.parseCoverageReport(allocator, cov_opts.output_dir) catch |err| {
108122 std.debug.print("Warning: Could not parse coverage report: {any}\n", .{err});
@@ -122,6 +136,10 @@ pub fn runDiscoveredTests(
122136
123137 std.debug.print("\n", .{});
124138 std.debug.print("Test Summary:\n", .{});
139 if (options.shard) |shard| {
140 std.debug.print(" Shard: {}/{}\n", .{ shard.index, shard.count });
141 std.debug.print(" Selected: {} of {} files\n", .{ selected_files, discovered.files.items.len });
142 }
125143 std.debug.print(" Files run: {}\n", .{files_run});
126144 std.debug.print(" Passed: {}\n", .{total_passed});
127145 std.debug.print(" Failed: {}\n", .{total_failed});
@@ -129,6 +147,21 @@ pub fn runDiscoveredTests(
129147 return total_failed == 0;
130148}
131149
150fn fileSelected(relative_path: []const u8, shard: ?sharding.ShardOptions) !bool {
151 return if (shard) |selection| selection.includes(relative_path) else true;
152}
153
154fn selectedFileCount(
155 discovered: *const discovery.DiscoveryResult,
156 shard: ?sharding.ShardOptions,
157) !usize {
158 var count: usize = 0;
159 for (discovered.files.items) |file| {
160 if (try fileSelected(file.relative_path, shard)) count += 1;
161 }
162 return count;
163}
164
132165/// Run a single test file using `zig test`
133166fn runTestFile(
134167 allocator: std.mem.Allocator,
@@ -207,6 +240,7 @@ test "LoaderOptions default values" {
207240 try std.testing.expectEqual(@as(?[]const u8, null), options.filter);
208241 try std.testing.expectEqual(false, options.verbose);
209242 try std.testing.expectEqual(true, options.use_colors);
243 try std.testing.expectEqual(@as(?sharding.ShardOptions, null), options.shard);
210244 try std.testing.expectEqual(@as(?coverage.CoverageOptions, null), options.coverage_options);
211245}
212246
@@ -256,3 +290,24 @@ test "LoaderOptions with coverage disabled" {
256290
257291 try std.testing.expectEqual(@as(?coverage.CoverageOptions, null), options.coverage_options);
258292}
293
294test "file sharding partitions discovered files exactly once" {
295 const allocator = std.testing.allocator;
296 var discovered = discovery.DiscoveryResult.init(allocator);
297 defer discovered.deinit();
298
299 try discovered.addFile("tests/api.test.zig", "api.test.zig", "api.test.zig");
300 try discovered.addFile("tests/db.test.zig", "db.test.zig", "db.test.zig");
301 try discovered.addFile("tests/math.test.zig", "math.test.zig", "math.test.zig");
302 try discovered.addFile("tests/string.test.zig", "string.test.zig", "string.test.zig");
303
304 var total_selected: usize = 0;
305 for (1..4) |index| {
306 total_selected += try selectedFileCount(
307 &discovered,
308 .{ .index = index, .count = 3 },
309 );
310 }
311
312 try std.testing.expectEqual(discovered.files.items.len, total_selected);
313}
tests/cli_test.zigmodified+23-0
Changes to tests/cli_test.zig
@@ -24,12 +24,33 @@ test "CLI rejects missing and invalid values" {
2424 try std.testing.expectError(ztf.cli.CLIError.InvalidArgument, invalid.parse(&.{ "zig-test", "--jobs", "0" }));
2525}
2626
27test "CLI parses and validates one-based sharding" {
28 var cli = ztf.CLI.init(std.testing.allocator);
29 try cli.parse(&.{ "zig-test", "--shard-index", "2", "--shard-count", "4" });
30 try std.testing.expectEqual(@as(?usize, 2), cli.options.shard_index);
31 try std.testing.expectEqual(@as(?usize, 4), cli.options.shard_count);
32
33 var missing_count = ztf.CLI.init(std.testing.allocator);
34 try std.testing.expectError(
35 ztf.cli.CLIError.InvalidArgument,
36 missing_count.parse(&.{ "zig-test", "--shard-index", "1" }),
37 );
38
39 var out_of_range = ztf.CLI.init(std.testing.allocator);
40 try std.testing.expectError(
41 ztf.cli.CLIError.InvalidArgument,
42 out_of_range.parse(&.{ "zig-test", "--shard-index", "3", "--shard-count", "2" }),
43 );
44}
45
2746test "CLI flags override configuration defaults" {
2847 var cli = ztf.CLI.init(std.testing.allocator);
2948 var config = ztf.TestConfig{};
3049 config.test_options.test_dir = "configured-tests";
3150 config.test_options.filter = "configured-filter";
3251 config.test_options.recursive = false;
52 config.sharding.index = 1;
53 config.sharding.count = 3;
3354 config.reporter.verbose = true;
3455
3556 try cli.applyConfig(config);
@@ -39,4 +60,6 @@ test "CLI flags override configuration defaults" {
3960 try std.testing.expectEqualStrings("cli-filter", cli.options.filter.?);
4061 try std.testing.expect(cli.options.no_recursive);
4162 try std.testing.expect(cli.options.verbose);
63 try std.testing.expectEqual(@as(?usize, 1), cli.options.shard_index);
64 try std.testing.expectEqual(@as(?usize, 3), cli.options.shard_count);
4265}
tests/fixtures/zig-test.jsonmodified+4-0
Changes to tests/fixtures/zig-test.json
@@ -4,5 +4,9 @@
44 "pattern": "filter_fixture.zig",
55 "recursive": false,
66 "filter": "selected test passes"
7 },
8 "sharding": {
9 "index": 1,
10 "count": 1
711 }
812}