From b8e568504e1dca38273a5845ac20d056cd7db5bb Mon Sep 17 00:00:00 2001 From: mlugg Date: Fri, 10 Jan 2025 10:48:52 +0000 Subject: [PATCH] std.Build: extend `test_runner` option to specify whether runner uses `std.zig.Server` The previous logic here was trying to assume that custom test runners never used `std.zig.Server` to communicate with the build runner; however, it was flawed, because modifying the `test_runner` field on `Step.Compile` would not update this flag. That might have been intentional (allowing a way for the user to specify a custom test runner which *does* use the compiler server protocol), but if so, it was a flawed API, since it was too easy to update one field without updating the other. Instead, bundle these two pieces of state into a new type `std.Build.Step.Compile.TestRunner`. When passing a custom test runner, you are now *provided* to specify whether it is a "simple" runner, or whether it uses the compiler server protocol. This is a breaking change, but is unlikely to affect many people, since custom test runners are seldom used in the wild. --- lib/std/Build.zig | 7 +++-- lib/std/Build/Step/Compile.zig | 27 ++++++++++++------- .../test_runner_module_imports/build.zig | 5 +++- test/standalone/test_runner_path/build.zig | 5 +++- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 636b5bbe4f..a9a5d18f6e 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -979,7 +979,7 @@ pub const TestOptions = struct { /// Deprecated; use `.filters = &.{filter}` instead of `.filter = filter`. filter: ?[]const u8 = null, filters: []const []const u8 = &.{}, - test_runner: ?LazyPath = null, + test_runner: ?Step.Compile.TestRunner = null, use_llvm: ?bool = null, use_lld: ?bool = null, zig_lib_dir: ?LazyPath = null, @@ -1136,9 +1136,8 @@ pub fn addRunArtifact(b: *Build, exe: *Step.Compile) *Step.Run { run_step.addArtifactArg(exe); } - if (exe.test_server_mode) { - run_step.enableTestRunnerMode(); - } + const test_server_mode = if (exe.test_runner) |r| r.mode == .server else true; + if (test_server_mode) run_step.enableTestRunnerMode(); } else { run_step.addArtifactArg(exe); } diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index 511d37d458..7daf10c68c 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -56,8 +56,7 @@ global_base: ?u64 = null, zig_lib_dir: ?LazyPath, exec_cmd_args: ?[]const ?[]const u8, filters: []const []const u8, -test_runner: ?LazyPath, -test_server_mode: bool, +test_runner: ?TestRunner, wasi_exec_model: ?std.builtin.WasiExecModel = null, installed_headers: ArrayList(HeaderInstallation), @@ -268,7 +267,7 @@ pub const Options = struct { version: ?std.SemanticVersion = null, max_rss: usize = 0, filters: []const []const u8 = &.{}, - test_runner: ?LazyPath = null, + test_runner: ?TestRunner = null, use_llvm: ?bool = null, use_lld: ?bool = null, zig_lib_dir: ?LazyPath = null, @@ -347,6 +346,14 @@ pub const HeaderInstallation = union(enum) { } }; +pub const TestRunner = struct { + path: LazyPath, + /// Test runners can either be "simple", running tests when spawned and terminating when the + /// tests are complete, or they can use `std.zig.Server` over stdio to interact more closely + /// with the build system. + mode: enum { simple, server }, +}; + pub fn create(owner: *std.Build, options: Options) *Compile { const name = owner.dupe(options.name); if (mem.indexOf(u8, name, "/") != null or mem.indexOf(u8, name, "\\") != null) { @@ -411,8 +418,7 @@ pub fn create(owner: *std.Build, options: Options) *Compile { .zig_lib_dir = null, .exec_cmd_args = null, .filters = options.filters, - .test_runner = null, - .test_server_mode = options.test_runner == null, + .test_runner = null, // set below .rdynamic = false, .installed_path = null, .force_undefined_symbols = StringHashMap(void).init(owner.allocator), @@ -438,9 +444,12 @@ pub fn create(owner: *std.Build, options: Options) *Compile { lp.addStepDependencies(&compile.step); } - if (options.test_runner) |lp| { - compile.test_runner = lp.dupe(compile.step.owner); - lp.addStepDependencies(&compile.step); + if (options.test_runner) |runner| { + compile.test_runner = .{ + .path = runner.path.dupe(compile.step.owner), + .mode = runner.mode, + }; + runner.path.addStepDependencies(&compile.step); } // Only the PE/COFF format has a Resource Table which is where the manifest @@ -1399,7 +1408,7 @@ fn getZigArgs(compile: *Compile, fuzz: bool) ![][]const u8 { if (compile.test_runner) |test_runner| { try zig_args.append("--test-runner"); - try zig_args.append(test_runner.getPath2(b, step)); + try zig_args.append(test_runner.path.getPath2(b, step)); } for (b.debug_log_scopes) |log_scope| { diff --git a/test/standalone/test_runner_module_imports/build.zig b/test/standalone/test_runner_module_imports/build.zig index 6b24ef7124..ae65308331 100644 --- a/test/standalone/test_runner_module_imports/build.zig +++ b/test/standalone/test_runner_module_imports/build.zig @@ -13,7 +13,10 @@ pub fn build(b: *std.Build) void { const t = b.addTest(.{ .root_module = test_mod, - .test_runner = b.path("test_runner/main.zig"), + .test_runner = .{ + .path = b.path("test_runner/main.zig"), + .mode = .simple, + }, }); const test_step = b.step("test", "Run unit tests"); diff --git a/test/standalone/test_runner_path/build.zig b/test/standalone/test_runner_path/build.zig index 1666031ba3..f2fdbfb0e0 100644 --- a/test/standalone/test_runner_path/build.zig +++ b/test/standalone/test_runner_path/build.zig @@ -8,7 +8,10 @@ pub fn build(b: *std.Build) void { .target = b.graph.host, .root_source_file = b.path("test.zig"), }) }); - test_exe.test_runner = b.path("test_runner.zig"); + test_exe.test_runner = .{ + .path = b.path("test_runner.zig"), + .mode = .simple, + }; const test_run = b.addRunArtifact(test_exe); test_step.dependOn(&test_run.step);