From 5523e2061b37de2b884c1c53fdcefb823755c7d0 Mon Sep 17 00:00:00 2001 From: Martin Wickham Date: Wed, 11 May 2022 02:22:13 -0500 Subject: [PATCH 1/2] Move std.testing.zig_exe_path into build options --- build.zig | 5 +++++ lib/std/process.zig | 1 - lib/std/testing.zig | 4 ---- lib/test_runner.zig | 17 ----------------- src/main.zig | 23 ++--------------------- src/test.zig | 8 +++++--- 6 files changed, 12 insertions(+), 46 deletions(-) diff --git a/build.zig b/build.zig index fc89b54023..2422a2a94c 100644 --- a/build.zig +++ b/build.zig @@ -46,6 +46,11 @@ pub fn build(b: *Builder) !void { test_cases.addPackagePath("test_cases", "test/cases.zig"); test_cases.single_threaded = single_threaded; + const test_options = b.addOptions(); + test_options.addOption([]const u8, "zig_exe_path", b.zig_exe); + test_cases.addOptions("test_options", test_options); + test_cases.step.dependOn(&test_options.step); + const fmt_build_zig = b.addFmt(&[_][]const u8{"build.zig"}); const skip_debug = b.option(bool, "skip-debug", "Main test suite skips debug builds") orelse false; diff --git a/lib/std/process.zig b/lib/std/process.zig index d8604edab8..2c51ec2d60 100644 --- a/lib/std/process.zig +++ b/lib/std/process.zig @@ -822,7 +822,6 @@ test "args iterator" { const given_suffix = std.fs.path.basename(prog_name); try testing.expect(mem.eql(u8, expected_suffix, given_suffix)); - try testing.expect(it.skip()); // Skip over zig_exe_path, passed to the test runner try testing.expect(it.next() == null); try testing.expect(!it.skip()); } diff --git a/lib/std/testing.zig b/lib/std/testing.zig index 67f6b33122..c71e2d39c9 100644 --- a/lib/std/testing.zig +++ b/lib/std/testing.zig @@ -22,10 +22,6 @@ pub var base_allocator_instance = std.heap.FixedBufferAllocator.init(""); /// TODO https://github.com/ziglang/zig/issues/5738 pub var log_level = std.log.Level.warn; -/// This is available to any test that wants to execute Zig in a child process. -/// It will be the same executable that is running `zig test`. -pub var zig_exe_path: []const u8 = undefined; - /// This function is intended to be used only in tests. It prints diagnostics to stderr /// and then returns a test failure error when actual_error_union is not expected_error. pub fn expectError(expected_error: anyerror, actual_error_union: anytype) !void { diff --git a/lib/test_runner.zig b/lib/test_runner.zig index 8a3c1f0ca7..aafaf1b073 100644 --- a/lib/test_runner.zig +++ b/lib/test_runner.zig @@ -6,29 +6,12 @@ pub const io_mode: io.Mode = builtin.test_io_mode; var log_err_count: usize = 0; -var args_buffer: [std.fs.MAX_PATH_BYTES + std.mem.page_size]u8 = undefined; -var args_allocator = std.heap.FixedBufferAllocator.init(&args_buffer); - -fn processArgs() void { - const args = std.process.argsAlloc(args_allocator.allocator()) catch { - @panic("Too many bytes passed over the CLI to the test runner"); - }; - if (args.len != 2) { - const self_name = if (args.len >= 1) args[0] else if (builtin.os.tag == .windows) "test.exe" else "test"; - const zig_ext = if (builtin.os.tag == .windows) ".exe" else ""; - std.debug.print("Usage: {s} path/to/zig{s}\n", .{ self_name, zig_ext }); - @panic("Wrong number of command line arguments"); - } - std.testing.zig_exe_path = args[1]; -} - pub fn main() void { if (builtin.zig_backend != .stage1 and (builtin.zig_backend != .stage2_llvm or builtin.cpu.arch == .wasm32)) { return main2() catch @panic("test failure"); } - processArgs(); const test_fn_list = builtin.test_functions; var ok_count: usize = 0; var skip_count: usize = 0; diff --git a/src/main.zig b/src/main.zig index d5a3bce82d..a3b15c34af 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3057,7 +3057,6 @@ fn buildOutputType( gpa, arena, test_exec_args.items, - self_exe_path, arg_mode, target_info, watch, @@ -3129,7 +3128,6 @@ fn buildOutputType( gpa, arena, test_exec_args.items, - self_exe_path, arg_mode, target_info, watch, @@ -3154,7 +3152,6 @@ fn buildOutputType( gpa, arena, test_exec_args.items, - self_exe_path, arg_mode, target_info, watch, @@ -3233,7 +3230,6 @@ fn runOrTest( gpa: Allocator, arena: Allocator, test_exec_args: []const ?[]const u8, - self_exe_path: []const u8, arg_mode: ArgMode, target_info: std.zig.system.NativeTargetInfo, watch: bool, @@ -3253,25 +3249,10 @@ fn runOrTest( defer argv.deinit(); if (test_exec_args.len == 0) { - // when testing pass the zig_exe_path to argv - if (arg_mode == .zig_test) - try argv.appendSlice(&[_][]const u8{ - exe_path, self_exe_path, - }) - // when running just pass the current exe - else - try argv.appendSlice(&[_][]const u8{ - exe_path, - }); + try argv.append(exe_path); } else { for (test_exec_args) |arg| { - if (arg) |a| { - try argv.append(a); - } else { - try argv.appendSlice(&[_][]const u8{ - exe_path, self_exe_path, - }); - } + try argv.append(arg orelse exe_path); } } if (runtime_args_start) |i| { diff --git a/src/test.zig b/src/test.zig index f26c65f3f8..49f229d221 100644 --- a/src/test.zig +++ b/src/test.zig @@ -1329,6 +1329,8 @@ pub const TestContext = struct { &[_][]const u8{ tmp_dir_path, "zig-cache" }, ); + const zig_exe_path = @import("test_options").zig_exe_path; + for (case.files.items) |file| { try tmp.dir.writeFile(file.path, file.src); } @@ -1351,7 +1353,7 @@ pub const TestContext = struct { try tmp.dir.writeFile(tmp_src_path, update.src); var zig_args = std.ArrayList([]const u8).init(arena); - try zig_args.append(std.testing.zig_exe_path); + try zig_args.append(zig_exe_path); if (case.is_test) { try zig_args.append("test"); @@ -1545,7 +1547,7 @@ pub const TestContext = struct { .link_libc = case.link_libc, .use_llvm = use_llvm, .use_stage1 = null, // We already handled stage1 tests - .self_exe_path = std.testing.zig_exe_path, + .self_exe_path = zig_exe_path, // TODO instead of turning off color, pass in a std.Progress.Node .color = .off, // TODO: force self-hosted linkers with stage2 backend to avoid LLD creeping in @@ -1795,7 +1797,7 @@ pub const TestContext = struct { continue :update; // Pass test. } try argv.appendSlice(&[_][]const u8{ - std.testing.zig_exe_path, + zig_exe_path, "run", "-cflags", "-std=c99", From 85b10eb07c27d021804473ee54a0b5942e4784d5 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 14 Sep 2022 14:56:45 -0700 Subject: [PATCH 2/2] ZIG_EXE envirnoment variable instead of testing build options No longer introduce build options for tests. Instead, ZIG_EXE environment variable is added to any invocation of `zig run` or `zig test`. The end result of this branch is the same: there is no longer a mandatory positional command line argument when invoking zig test binaries directly. --- build.zig | 7 +------ src/main.zig | 10 +++++++++- src/test.zig | 5 +++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/build.zig b/build.zig index 2422a2a94c..3bb5474ce3 100644 --- a/build.zig +++ b/build.zig @@ -40,17 +40,12 @@ pub fn build(b: *Builder) !void { const docs_step = b.step("docs", "Build documentation"); docs_step.dependOn(&docgen_cmd.step); - var test_cases = b.addTest("src/test.zig"); + const test_cases = b.addTest("src/test.zig"); test_cases.stack_size = stack_size; test_cases.setBuildMode(mode); test_cases.addPackagePath("test_cases", "test/cases.zig"); test_cases.single_threaded = single_threaded; - const test_options = b.addOptions(); - test_options.addOption([]const u8, "zig_exe_path", b.zig_exe); - test_cases.addOptions("test_options", test_options); - test_cases.step.dependOn(&test_options.step); - const fmt_build_zig = b.addFmt(&[_][]const u8{"build.zig"}); const skip_debug = b.option(bool, "skip-debug", "Main test suite skips debug builds") orelse false; diff --git a/src/main.zig b/src/main.zig index a3b15c34af..4e24b3d789 100644 --- a/src/main.zig +++ b/src/main.zig @@ -3057,6 +3057,7 @@ fn buildOutputType( gpa, arena, test_exec_args.items, + self_exe_path, arg_mode, target_info, watch, @@ -3128,6 +3129,7 @@ fn buildOutputType( gpa, arena, test_exec_args.items, + self_exe_path, arg_mode, target_info, watch, @@ -3152,6 +3154,7 @@ fn buildOutputType( gpa, arena, test_exec_args.items, + self_exe_path, arg_mode, target_info, watch, @@ -3230,6 +3233,7 @@ fn runOrTest( gpa: Allocator, arena: Allocator, test_exec_args: []const ?[]const u8, + self_exe_path: []const u8, arg_mode: ArgMode, target_info: std.zig.system.NativeTargetInfo, watch: bool, @@ -3258,16 +3262,20 @@ fn runOrTest( if (runtime_args_start) |i| { try argv.appendSlice(all_args[i..]); } + var env_map = try std.process.getEnvMap(arena); + try env_map.put("ZIG_EXE", self_exe_path); + // We do not execve for tests because if the test fails we want to print // the error message and invocation below. if (std.process.can_execv and arg_mode == .run and !watch) { // execv releases the locks; no need to destroy the Compilation here. - const err = std.process.execv(gpa, argv.items); + const err = std.process.execve(gpa, argv.items, &env_map); try warnAboutForeignBinaries(arena, arg_mode, target_info, link_libc); const cmd = try std.mem.join(arena, " ", argv.items); fatal("the following command failed to execve with '{s}':\n{s}", .{ @errorName(err), cmd }); } else if (std.process.can_spawn) { var child = std.ChildProcess.init(argv.items, gpa); + child.env_map = &env_map; child.stdin_behavior = .Inherit; child.stdout_behavior = .Inherit; child.stderr_behavior = .Inherit; diff --git a/src/test.zig b/src/test.zig index 49f229d221..b95a66c11f 100644 --- a/src/test.zig +++ b/src/test.zig @@ -1214,6 +1214,7 @@ pub const TestContext = struct { fn run(self: *TestContext) !void { const host = try std.zig.system.NativeTargetInfo.detect(.{}); + const zig_exe_path = try std.process.getEnvVarOwned(self.arena, "ZIG_EXE"); var progress = std.Progress{}; const root_node = progress.start("compiler", self.cases.items.len); @@ -1272,6 +1273,7 @@ pub const TestContext = struct { &prg_node, case.*, zig_lib_directory, + zig_exe_path, &aux_thread_pool, global_cache_directory, host, @@ -1298,6 +1300,7 @@ pub const TestContext = struct { root_node: *std.Progress.Node, case: Case, zig_lib_directory: Compilation.Directory, + zig_exe_path: []const u8, thread_pool: *ThreadPool, global_cache_directory: Compilation.Directory, host: std.zig.system.NativeTargetInfo, @@ -1329,8 +1332,6 @@ pub const TestContext = struct { &[_][]const u8{ tmp_dir_path, "zig-cache" }, ); - const zig_exe_path = @import("test_options").zig_exe_path; - for (case.files.items) |file| { try tmp.dir.writeFile(file.path, file.src); }