ReviewOS

also looking at this

zig-utils/zig-test-framework

fix: honor discovery-mode CLI options

#17
Merged glennmichael123 wants to merge codex/issue-2-discovery-cli into main
10 files +192 -24
README.mdmodified+7-0
Changes to README.md
@@ -94,8 +94,15 @@ This will automatically discover and run all `*.test.zig` files in the `tests` d
9494- `--pattern <pattern>` - File pattern to match (default: `*.test.zig`)
9595- `--no-recursive` - Disable recursive directory search
9696- `--bail` - Stop on first failure
97- `--filter <text>` / `--grep <text>` - Run only Zig tests whose names contain the text
98- `--no-color` - Disable color in child Zig test processes
9799- `--verbose` - Show detailed output
98100
101Discovery mode rejects options that it cannot honor yet instead of silently
102ignoring them. Reporter selection, quiet output, timeouts, parallel jobs,
103snapshot updates, memory profiling, configuration files, JUnit output, and the
104web UI require the shared execution pipeline planned for future releases.
105
99106**Examples:**
100107
101108```bash
build.zigmodified+3-0
Changes to build.zig
@@ -130,6 +130,9 @@ pub fn build(b: *std.Build) void {
130130 .target = target,
131131 .optimize = optimize,
132132 .link_libc = true,
133 .imports = &.{
134 .{ .name = "zig_test_framework", .module = lib_module },
135 },
133136 }),
134137 });
135138 const run_filter_tests = b.addRunArtifact(filter_tests);
src/cli.zigmodified+83-1
Changes to src/cli.zig
@@ -11,7 +11,7 @@ pub const CLIOptions = struct {
1111 quiet: bool = false,
1212 no_color: bool = false,
1313 // Test discovery options
14 test_dir: ?[]const u8 = null,
14 test_dir: ?[]const u8 = ".",
1515 pattern: []const u8 = "*.test.zig",
1616 no_recursive: bool = false,
1717 // Coverage options
@@ -140,6 +140,12 @@ pub const CLI = struct {
140140 }
141141 i += 1;
142142 self.options.coverage_tool = args[i];
143 if (!std.mem.eql(u8, self.options.coverage_tool, "kcov") and
144 !std.mem.eql(u8, self.options.coverage_tool, "grindcov"))
145 {
146 std.debug.print("Error: --coverage-tool must be 'kcov' or 'grindcov'\n", .{});
147 return CLIError.InvalidArgument;
148 }
143149 } else if (std.mem.eql(u8, arg, "--parallel") or std.mem.eql(u8, arg, "-p")) {
144150 self.options.parallel = true;
145151 } else if (std.mem.eql(u8, arg, "--jobs") or std.mem.eql(u8, arg, "-j")) {
@@ -153,6 +159,10 @@ pub const CLI = struct {
153159 std.debug.print("Error: --jobs must be a valid number\n", .{});
154160 return CLIError.InvalidArgument;
155161 };
162 if (self.options.jobs.? == 0) {
163 std.debug.print("Error: --jobs must be greater than zero\n", .{});
164 return CLIError.InvalidArgument;
165 }
156166 } else if (std.mem.eql(u8, arg, "--ui")) {
157167 self.options.ui = true;
158168 } else if (std.mem.eql(u8, arg, "--ui-port")) {
@@ -245,6 +255,31 @@ pub const CLI = struct {
245255 }
246256 }
247257
258 /// Return the first option that discovery mode cannot currently honor.
259 /// Discovery must reject these options explicitly instead of silently
260 /// behaving differently from programmatic mode.
261 pub fn unsupportedDiscoveryOption(self: Self) ?[]const u8 {
262 if (!self.options.coverage and !std.mem.eql(u8, self.options.coverage_dir, "coverage")) return "--coverage-dir";
263 if (!self.options.coverage and !std.mem.eql(u8, self.options.coverage_tool, "kcov")) return "--coverage-tool";
264 if (!self.options.watch and self.options.watch_debounce != 300) return "--watch-debounce";
265 if (!self.options.ui and self.options.ui_port != 8080) return "--ui-port";
266 if (!self.options.ui and !std.mem.eql(u8, self.options.ui_host, "127.0.0.1")) return "--ui-host";
267 if (self.options.reporter != .spec) return "--reporter";
268 if (self.options.quiet) return "--quiet";
269 if (self.options.parallel) return "--parallel";
270 if (self.options.jobs != null) return "--jobs";
271 if (self.options.update_snapshots) return "--update-snapshots";
272 if (!std.mem.eql(u8, self.options.snapshot_dir, ".snapshots")) return "--snapshot-dir";
273 if (self.options.profile_memory) return "--profile-memory";
274 if (self.options.memory_threshold != 0) return "--memory-threshold";
275 if (self.options.fail_on_leak) return "--fail-on-leak";
276 if (self.options.config != null) return "--config";
277 if (self.options.junit_output != null) return "--junit-output";
278 if (self.options.timeout != null) return "--timeout";
279 if (self.options.ui) return "--ui";
280 return null;
281 }
282
248283 /// Print help message
249284 pub fn printHelp(self: Self) void {
250285 _ = self;
@@ -467,6 +502,7 @@ test "CLI default values" {
467502 try std.testing.expectEqualStrings("coverage", cli.options.coverage_dir);
468503 try std.testing.expectEqualStrings("kcov", cli.options.coverage_tool);
469504 try std.testing.expectEqualStrings("*.test.zig", cli.options.pattern);
505 try std.testing.expectEqualStrings(".", cli.options.test_dir.?);
470506 try std.testing.expect(!cli.options.ui);
471507 try std.testing.expectEqual(@as(u16, 8080), cli.options.ui_port);
472508 try std.testing.expectEqualStrings("127.0.0.1", cli.options.ui_host);
@@ -531,6 +567,52 @@ test "CLI parse timeout" {
531567 try std.testing.expectEqual(@as(?u64, 5000), cli.options.timeout);
532568}
533569
570test "CLI rejects invalid coverage tool" {
571 var cli = CLI.init(std.testing.allocator);
572 const args = [_][]const u8{ "zig-test", "--coverage-tool", "unknown" };
573 try std.testing.expectError(CLIError.InvalidArgument, cli.parse(&args));
574}
575
576test "CLI rejects zero jobs" {
577 var cli = CLI.init(std.testing.allocator);
578 const args = [_][]const u8{ "zig-test", "--jobs", "0" };
579 try std.testing.expectError(CLIError.InvalidArgument, cli.parse(&args));
580}
581
582test "discovery accepts implemented options" {
583 var cli = CLI.init(std.testing.allocator);
584 const args = [_][]const u8{
585 "zig-test",
586 "--test-dir",
587 "tests",
588 "--filter",
589 "math",
590 "--no-color",
591 "--bail",
592 "--verbose",
593 };
594 try cli.parse(&args);
595 try std.testing.expectEqual(@as(?[]const u8, null), cli.unsupportedDiscoveryOption());
596}
597
598test "discovery identifies unsupported options" {
599 var cli = CLI.init(std.testing.allocator);
600 const args = [_][]const u8{ "zig-test", "--test-dir", "tests", "--reporter", "json" };
601 try cli.parse(&args);
602 try std.testing.expectEqualStrings("--reporter", cli.unsupportedDiscoveryOption().?);
603}
604
605test "discovery reports timeout and parallel options as unsupported" {
606 var cli = CLI.init(std.testing.allocator);
607
608 cli.options.timeout = 1000;
609 try std.testing.expectEqualStrings("--timeout", cli.unsupportedDiscoveryOption().?);
610
611 cli.options.timeout = null;
612 cli.options.parallel = true;
613 try std.testing.expectEqualStrings("--parallel", cli.unsupportedDiscoveryOption().?);
614}
615
534616test "CLI parse update-snapshots short flag" {
535617 var cli = CLI.init(std.testing.allocator);
536618 const args = [_][]const u8{ "zig-test", "-u" };
src/coverage.zigmodified+12-0
Changes to src/coverage.zig
@@ -168,6 +168,16 @@ pub fn runTestWithCoverage(
168168 allocator: std.mem.Allocator,
169169 test_file_path: []const u8,
170170 options: CoverageOptions,
171) !bool {
172 return runTestWithCoverageArgs(allocator, test_file_path, &.{}, options);
173}
174
175/// Run a test file with coverage while forwarding Zig test arguments.
176pub fn runTestWithCoverageArgs(
177 allocator: std.mem.Allocator,
178 test_file_path: []const u8,
179 test_args: []const []const u8,
180 options: CoverageOptions,
171181) !bool {
172182 if (!options.enabled) {
173183 return error.CoverageNotEnabled;
@@ -189,6 +199,7 @@ pub fn runTestWithCoverage(
189199 try argv.append(allocator, "zig");
190200 try argv.append(allocator, "test");
191201 try argv.append(allocator, test_file_path);
202 try argv.appendSlice(allocator, test_args);
192203
193204 const term = try compat.spawnAndWait(allocator, argv.items, .Inherit, .Inherit);
194205 return switch (term) {
@@ -210,6 +221,7 @@ pub fn runTestWithCoverage(
210221 try argv.append(allocator, "zig");
211222 try argv.append(allocator, "test");
212223 try argv.append(allocator, test_file_path);
224 try argv.appendSlice(allocator, test_args);
213225
214226 switch (options.tool) {
215227 .kcov => {
src/lib.zigmodified+1-0
Changes to src/lib.zig
@@ -116,6 +116,7 @@ pub const CoverageResult = coverage.CoverageResult;
116116pub const CoverageTool = coverage.CoverageTool;
117117pub const runWithCoverage = coverage.runWithCoverage;
118118pub const runTestWithCoverage = coverage.runTestWithCoverage;
119pub const runTestWithCoverageArgs = coverage.runTestWithCoverageArgs;
119120pub const parseCoverageReport = coverage.parseCoverageReport;
120121pub const printCoverageSummary = coverage.printCoverageSummary;
121122pub const isCoverageToolAvailable = coverage.isCoverageToolAvailable;
src/main.zigmodified+13-1
Changes to src/main.zig
@@ -74,6 +74,16 @@ pub fn main(init: std.process.Init.Minimal) !void {
7474 // Check if we should use test discovery or programmatic tests
7575 const use_discovery = cli_parser.options.test_dir != null;
7676
77 if (use_discovery) {
78 if (cli_parser.unsupportedDiscoveryOption()) |option| {
79 std.debug.print(
80 "Error: {s} cannot be honored in test discovery mode\n",
81 .{option},
82 );
83 std.process.exit(2);
84 }
85 }
86
7787 // Start UI server if requested
7888 var ui_server: ?lib.UIServer = null;
7989 var ui_thread: ?std.Thread = null;
@@ -176,6 +186,7 @@ pub fn main(init: std.process.Init.Minimal) !void {
176186 .bail = cli_parser.options.bail,
177187 .filter = cli_parser.options.filter,
178188 .verbose = cli_parser.options.verbose,
189 .use_colors = !cli_parser.options.no_color,
179190 .coverage_options = cov_opts,
180191 .ui_server = if (ui_server) |*server| server else null,
181192 };
@@ -192,7 +203,7 @@ pub fn main(init: std.process.Init.Minimal) !void {
192203 .recursive = !cli_parser.options.no_recursive,
193204 };
194205
195 std.debug.print("Discovering tests in '{s}' with pattern '*{s}'...\n\n", .{ discovery_options.root_path, discovery_options.pattern });
206 std.debug.print("Discovering tests in '{s}' with pattern '{s}'...\n\n", .{ discovery_options.root_path, discovery_options.pattern });
196207
197208 var discovered = try lib.discoverTests(allocator, discovery_options);
198209 defer discovered.deinit();
@@ -213,6 +224,7 @@ pub fn main(init: std.process.Init.Minimal) !void {
213224 .bail = cli_parser.options.bail,
214225 .filter = cli_parser.options.filter,
215226 .verbose = cli_parser.options.verbose,
227 .use_colors = !cli_parser.options.no_color,
216228 .coverage_options = cov_opts,
217229 .ui_server = if (ui_server) |*server| server else null,
218230 };
src/test_loader.zigmodified+45-3
Changes to src/test_loader.zig
@@ -12,6 +12,8 @@ pub const LoaderOptions = struct {
1212 filter: ?[]const u8 = null,
1313 /// Whether to show verbose output
1414 verbose: bool = false,
15 /// Whether child Zig test processes may emit colors
16 use_colors: bool = true,
1517 /// Coverage options
1618 coverage_options: ?coverage.CoverageOptions = null,
1719 /// UI server for real-time updates
@@ -133,27 +135,48 @@ fn runTestFile(
133135 file_path: []const u8,
134136 options: LoaderOptions,
135137) !bool {
138 var test_args: std.ArrayList([]const u8) = .empty;
139 defer test_args.deinit(allocator);
140
141 try appendTestArgs(allocator, &test_args, options);
142
136143 // If coverage is enabled, use coverage.runTestWithCoverage
137144 if (options.coverage_options) |cov_opts| {
138145 if (cov_opts.enabled) {
139146 // Disable clean for individual test runs since we cleaned once at the start
140147 var modified_opts = cov_opts;
141148 modified_opts.clean = false;
142 return coverage.runTestWithCoverage(allocator, file_path, modified_opts) catch |err| {
149 return coverage.runTestWithCoverageArgs(allocator, file_path, test_args.items, modified_opts) catch |err| {
143150 std.debug.print("Warning: Coverage failed for {s}: {any}\n", .{ file_path, err });
144151 // Fall back to running without coverage
145 return runTestFileWithoutCoverage(allocator, file_path);
152 return runTestFileWithoutCoverage(allocator, file_path, test_args.items);
146153 };
147154 }
148155 }
149156
150 return runTestFileWithoutCoverage(allocator, file_path);
157 return runTestFileWithoutCoverage(allocator, file_path, test_args.items);
158}
159
160fn appendTestArgs(
161 allocator: std.mem.Allocator,
162 args: *std.ArrayList([]const u8),
163 options: LoaderOptions,
164) !void {
165 if (options.filter) |filter| {
166 try args.append(allocator, "--test-filter");
167 try args.append(allocator, filter);
168 }
169 if (!options.use_colors) {
170 try args.append(allocator, "--color");
171 try args.append(allocator, "off");
172 }
151173}
152174
153175/// Run a single test file without coverage
154176fn runTestFileWithoutCoverage(
155177 allocator: std.mem.Allocator,
156178 file_path: []const u8,
179 test_args: []const []const u8,
157180) !bool {
158181 // Build zig test command
159182 var argv: std.ArrayList([]const u8) = .empty;
@@ -162,6 +185,7 @@ fn runTestFileWithoutCoverage(
162185 try argv.append(allocator, "zig");
163186 try argv.append(allocator, "test");
164187 try argv.append(allocator, file_path);
188 try argv.appendSlice(allocator, test_args);
165189
166190 // Run the test
167191 const term = try compat.spawnAndWait(allocator, argv.items, .Inherit, .Inherit);
@@ -182,9 +206,27 @@ test "LoaderOptions default values" {
182206 try std.testing.expectEqual(false, options.bail);
183207 try std.testing.expectEqual(@as(?[]const u8, null), options.filter);
184208 try std.testing.expectEqual(false, options.verbose);
209 try std.testing.expectEqual(true, options.use_colors);
185210 try std.testing.expectEqual(@as(?coverage.CoverageOptions, null), options.coverage_options);
186211}
187212
213test "test command options forward filter and color" {
214 const allocator = std.testing.allocator;
215 var args: std.ArrayList([]const u8) = .empty;
216 defer args.deinit(allocator);
217
218 try appendTestArgs(allocator, &args, .{
219 .filter = "selected test",
220 .use_colors = false,
221 });
222
223 try std.testing.expectEqual(@as(usize, 4), args.items.len);
224 try std.testing.expectEqualStrings("--test-filter", args.items[0]);
225 try std.testing.expectEqualStrings("selected test", args.items[1]);
226 try std.testing.expectEqualStrings("--color", args.items[2]);
227 try std.testing.expectEqualStrings("off", args.items[3]);
228}
229
188230test "LoaderOptions with custom values" {
189231 const cov_opts = coverage.CoverageOptions{
190232 .enabled = true,
src/timeout.zigmodified+4-6
Changes to src/timeout.zig
@@ -403,14 +403,12 @@ test "TimeoutContext extension" {
403403 // Extend timeout
404404 try context.extend(100);
405405
406 compat.sleep(120 * std.time.ns_per_ms);
407
408 // Should not be timed out (extended to 200ms total)
406 // Move the clock origin instead of relying on scheduler timing. Under a
407 // loaded CI runner, a nominal 120ms sleep can exceed the 200ms deadline.
408 context.start_time = compat.milliTimestamp() - 120;
409409 try std.testing.expect(!context.isTimedOut());
410410
411 compat.sleep(100 * std.time.ns_per_ms);
412
413 // Should be timed out now
411 context.start_time = compat.milliTimestamp() - 220;
414412 try std.testing.expect(context.isTimedOut());
415413}
416414
tests/filter_test.zigmodified+15-13
Changes to tests/filter_test.zig
@@ -1,19 +1,21 @@
11const std = ;
2const ztf = ;
23
3// Placeholder tests for filter module
4// TODO: Implement comprehensive tests for test filtering
4test "discovery forwards test-name filters to zig test" {
5 const allocator = std.testing.allocator;
6 var discovered = ztf.DiscoveryResult.init(allocator);
7 defer discovered.deinit();
58
6test "filter by test name" {
7 // Test filtering tests by name
8 try std.testing.expect(true);
9}
9 try discovered.addFile(
10 "tests/fixtures/filter_fixture.zig",
11 "fixtures/filter_fixture.zig",
12 "filter_fixture.zig",
13 );
1014
11test "filter by pattern" {
12 // Test filtering tests by pattern
13 try std.testing.expect(true);
14}
15 const passed = try ztf.runDiscoveredTests(allocator, &discovered, .{
16 .filter = "selected test passes",
17 .use_colors = false,
18 });
1519
16test "filter combination" {
17 // Test combined filters
18 try std.testing.expect(true);
20 try std.testing.expect(passed);
1921}
tests/fixtures/filter_fixture.zigadded+9-0
Changes to tests/fixtures/filter_fixture.zig
@@ -0,0 +1,9 @@
1const std = ;
2
3test "selected test passes" {
4 try std.testing.expect(true);
5}
6
7test "unselected test fails" {
8 return error.ShouldHaveBeenFiltered;
9}