Skip to content

Commit

Permalink
make the build runner and test runner talk to each other
Browse files Browse the repository at this point in the history 
std.Build.addTest creates a CompileStep as before, however, this kind of
step no longer actually runs the unit tests. Instead it only compiles
it, and one must additionally create a RunStep from the CompileStep in
order to actually run the tests.

RunStep gains integration with the default test runner, which now
supports the standard --listen=- argument in order to communicate over
stdin and stdout. It also reports test statistics; how many passed,
failed, and leaked, as well as directly associating the relevant stderr
with the particular test name that failed.

This separation of CompileStep and RunStep means that
`CompileStep.Kind.test_exe` is no longer needed, and therefore has been
removed in this commit.

 * build runner: show unit test statistics in build summary
 * added Step.writeManifest since many steps want to treat it as a
   warning and emit the same message if it fails.
 * RunStep: fixed error message that prints the failed command printing
   the original argv and not the adjusted argv in case an interpreter
   was used.
 * RunStep: fixed not passing the command line arguments to the
   interpreter.
 * move src/Server.zig to std.zig.Server so that the default test runner
   can use it.
 * the simpler test runner function which is used by work-in-progress
   backends now no longer prints to stderr, which is necessary in order
   for the build runner to not print the stderr as a warning message.
  • Loading branch information
@andrewrk
andrewrk committed Mar 12, 2023
1 parent a67f410 commit 1b8da32
Show file tree
Hide file tree
Showing 30 changed files with 780 additions and 373 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ set(ZIG_STAGE2_SOURCES
"${CMAKE_SOURCE_DIR}/lib/std/zig/c_builtins.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/Parse.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/render.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/Server.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/string_literal.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/system.zig"
"${CMAKE_SOURCE_DIR}/lib/std/zig/system/NativePaths.zig"
Expand Down Expand Up @@ -620,7 +621,6 @@ set(ZIG_STAGE2_SOURCES
"${CMAKE_SOURCE_DIR}/src/print_targets.zig"
"${CMAKE_SOURCE_DIR}/src/print_zir.zig"
"${CMAKE_SOURCE_DIR}/src/register_manager.zig"
"${CMAKE_SOURCE_DIR}/src/Server.zig"
"${CMAKE_SOURCE_DIR}/src/target.zig"
"${CMAKE_SOURCE_DIR}/src/tracy.zig"
"${CMAKE_SOURCE_DIR}/src/translate_c.zig"
Expand Down
59 changes: 57 additions & 2 deletions lib/build_runner.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,12 @@ fn runStepNames(
}
assert(run.memory_blocked_steps.items.len == 0);

var test_skip_count: usize = 0;
var test_fail_count: usize = 0;
var test_pass_count: usize = 0;
var test_leak_count: usize = 0;
var test_count: usize = 0;

var success_count: usize = 0;
var skipped_count: usize = 0;
var failure_count: usize = 0;
Expand All @@ -425,6 +431,12 @@ fn runStepNames(
defer compile_error_steps.deinit(gpa);

for (step_stack.keys()) |s| {
test_fail_count += s.test_results.fail_count;
test_skip_count += s.test_results.skip_count;
test_leak_count += s.test_results.leak_count;
test_pass_count += s.test_results.passCount();
test_count += s.test_results.test_count;

switch (s.state) {
.precheck_unstarted => unreachable,
.precheck_started => unreachable,
Expand Down Expand Up @@ -468,6 +480,11 @@ fn runStepNames(
if (skipped_count > 0) stderr.writer().print("; {d} skipped", .{skipped_count}) catch {};
if (failure_count > 0) stderr.writer().print("; {d} failed", .{failure_count}) catch {};

if (test_count > 0) stderr.writer().print("; {d}/{d} tests passed", .{ test_pass_count, test_count }) catch {};
if (test_skip_count > 0) stderr.writer().print("; {d} skipped", .{test_skip_count}) catch {};
if (test_fail_count > 0) stderr.writer().print("; {d} failed", .{test_fail_count}) catch {};
if (test_leak_count > 0) stderr.writer().print("; {d} leaked", .{test_leak_count}) catch {};

if (run.enable_summary == null) {
ttyconf.setColor(stderr, .Dim) catch {};
stderr.writeAll(" (disable with -fno-summary)") catch {};
Expand Down Expand Up @@ -566,6 +583,13 @@ fn printTreeStep(
try ttyconf.setColor(stderr, .Green);
if (s.result_cached) {
try stderr.writeAll(" cached");
} else if (s.test_results.test_count > 0) {
const pass_count = s.test_results.passCount();
try stderr.writer().print(" {d} passed", .{pass_count});
if (s.test_results.skip_count > 0) {
try ttyconf.setColor(stderr, .Yellow);
try stderr.writer().print(" {d} skipped", .{s.test_results.skip_count});
}
} else {
try stderr.writeAll(" success");
}
Expand Down Expand Up @@ -609,15 +633,46 @@ fn printTreeStep(
},

.failure => {
try ttyconf.setColor(stderr, .Red);
if (s.result_error_bundle.errorMessageCount() > 0) {
try ttyconf.setColor(stderr, .Red);
try stderr.writer().print(" {d} errors\n", .{
s.result_error_bundle.errorMessageCount(),
});
try ttyconf.setColor(stderr, .Reset);
} else if (!s.test_results.isSuccess()) {
try stderr.writer().print(" {d}/{d} passed", .{
s.test_results.passCount(), s.test_results.test_count,
});
if (s.test_results.fail_count > 0) {
try stderr.writeAll(", ");
try ttyconf.setColor(stderr, .Red);
try stderr.writer().print("{d} failed", .{
s.test_results.fail_count,
});
try ttyconf.setColor(stderr, .Reset);
}
if (s.test_results.skip_count > 0) {
try stderr.writeAll(", ");
try ttyconf.setColor(stderr, .Yellow);
try stderr.writer().print("{d} skipped", .{
s.test_results.skip_count,
});
try ttyconf.setColor(stderr, .Reset);
}
if (s.test_results.leak_count > 0) {
try stderr.writeAll(", ");
try ttyconf.setColor(stderr, .Red);
try stderr.writer().print("{d} leaked", .{
s.test_results.leak_count,
});
try ttyconf.setColor(stderr, .Reset);
}
try stderr.writeAll("\n");
} else {
try ttyconf.setColor(stderr, .Red);
try stderr.writeAll(" failure\n");
try ttyconf.setColor(stderr, .Reset);
}
try ttyconf.setColor(stderr, .Reset);
},
}

Expand Down
10 changes: 4 additions & 6 deletions lib/std/Build.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ pub fn addStaticLibrary(b: *Build, options: StaticLibraryOptions) *CompileStep {

pub const TestOptions = struct {
name: []const u8 = "test",
kind: CompileStep.Kind = .@"test",
root_source_file: FileSource,
target: CrossTarget = .{},
optimize: std.builtin.Mode = .Debug,
Expand All @@ -542,7 +541,7 @@ pub const TestOptions = struct {
pub fn addTest(b: *Build, options: TestOptions) *CompileStep {
return CompileStep.create(b, .{
.name = options.name,
.kind = options.kind,
.kind = .@"test",
.root_source_file = options.root_source_file,
.target = options.target,
.optimize = options.optimize,
Expand Down Expand Up @@ -626,16 +625,15 @@ pub fn addSystemCommand(self: *Build, argv: []const []const u8) *RunStep {
/// Creates a `RunStep` with an executable built with `addExecutable`.
/// Add command line arguments with methods of `RunStep`.
pub fn addRunArtifact(b: *Build, exe: *CompileStep) *RunStep {
assert(exe.kind == .exe or exe.kind == .test_exe);

// It doesn't have to be native. We catch that if you actually try to run it.
// Consider that this is declarative; the run step may not be run unless a user
// option is supplied.
const run_step = RunStep.create(b, b.fmt("run {s}", .{exe.name}));
run_step.addArtifactArg(exe);

if (exe.kind == .test_exe) {
run_step.addArg(b.zig_exe);
if (exe.kind == .@"test") {
run_step.stdio = .zig_test;
run_step.addArgs(&.{"--listen=-"});
}

if (exe.vcpkg_bin_path) |path| {
Expand Down
93 changes: 7 additions & 86 deletions lib/std/Build/CompileStep.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ pub const Kind = enum {
lib,
obj,
@"test",
test_exe,
};

pub const Linkage = enum { dynamic, static };
Expand Down Expand Up @@ -328,7 +327,7 @@ pub fn create(owner: *std.Build, options: Options) *CompileStep {
.exe => "zig build-exe",
.lib => "zig build-lib",
.obj => "zig build-obj",
.test_exe, .@"test" => "zig test",
.@"test" => "zig test",
},
name_adjusted,
@tagName(options.optimize),
Expand Down Expand Up @@ -410,7 +409,7 @@ fn computeOutFileNames(self: *CompileStep) void {
.output_mode = switch (self.kind) {
.lib => .Lib,
.obj => .Obj,
.exe, .@"test", .test_exe => .Exe,
.exe, .@"test" => .Exe,
},
.link_mode = if (self.linkage) |some| @as(std.builtin.LinkMode, switch (some) {
.dynamic => .Dynamic,
Expand Down Expand Up @@ -621,7 +620,7 @@ pub fn producesPdbFile(self: *CompileStep) bool {
if (!self.target.isWindows() and !self.target.isUefi()) return false;
if (self.target.getObjectFormat() == .c) return false;
if (self.strip == true) return false;
return self.isDynamicLibrary() or self.kind == .exe or self.kind == .test_exe;
return self.isDynamicLibrary() or self.kind == .exe or self.kind == .@"test";
}

pub fn linkLibC(self: *CompileStep) void {
Expand Down Expand Up @@ -850,19 +849,19 @@ fn linkSystemLibraryInner(self: *CompileStep, name: []const u8, opts: struct {

pub fn setNamePrefix(self: *CompileStep, text: []const u8) void {
const b = self.step.owner;
assert(self.kind == .@"test" or self.kind == .test_exe);
assert(self.kind == .@"test");
self.name_prefix = b.dupe(text);
}

pub fn setFilter(self: *CompileStep, text: ?[]const u8) void {
const b = self.step.owner;
assert(self.kind == .@"test" or self.kind == .test_exe);
assert(self.kind == .@"test");
self.filter = if (text) |t| b.dupe(t) else null;
}

pub fn setTestRunner(self: *CompileStep, path: ?[]const u8) void {
const b = self.step.owner;
assert(self.kind == .@"test" or self.kind == .test_exe);
assert(self.kind == .@"test");
self.test_runner = if (path) |p| b.dupePath(p) else null;
}

Expand Down Expand Up @@ -938,7 +937,7 @@ pub fn getOutputLibSource(self: *CompileStep) FileSource {
/// Returns the generated header file.
/// This function can only be called for libraries or object files which have `emit_h` set.
pub fn getOutputHSource(self: *CompileStep) FileSource {
assert(self.kind != .exe and self.kind != .test_exe and self.kind != .@"test");
assert(self.kind != .exe and self.kind != .@"test");
assert(self.emit_h);
return .{ .generated = &self.output_h_path_source };
}
Expand Down Expand Up @@ -1243,7 +1242,6 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
.exe => "build-exe",
.obj => "build-obj",
.@"test" => "test",
.test_exe => "test",
};
try zig_args.append(cmd);

Expand Down Expand Up @@ -1293,7 +1291,6 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {

.other_step => |other| switch (other.kind) {
.exe => @panic("Cannot link with an executable build artifact"),
.test_exe => @panic("Cannot link with an executable build artifact"),
.@"test" => @panic("Cannot link with a test"),
.obj => {
try zig_args.append(other.getOutputSource().getPath(b));
Expand Down Expand Up @@ -1661,83 +1658,7 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void {
try zig_args.append("--test-cmd-bin");
}
}
} else {
const need_cross_glibc = self.target.isGnuLibC() and transitive_deps.is_linking_libc;

switch (b.host.getExternalExecutor(self.target_info, .{
.qemu_fixes_dl = need_cross_glibc and b.glibc_runtimes_dir != null,
.link_libc = transitive_deps.is_linking_libc,
})) {
.native => {},
.bad_dl, .bad_os_or_cpu => {
try zig_args.append("--test-no-exec");
},
.rosetta => if (b.enable_rosetta) {
try zig_args.append("--test-cmd-bin");
} else {
try zig_args.append("--test-no-exec");
},
.qemu => |bin_name| ok: {
if (b.enable_qemu) qemu: {
const glibc_dir_arg = if (need_cross_glibc)
b.glibc_runtimes_dir orelse break :qemu
else
null;
try zig_args.append("--test-cmd");
try zig_args.append(bin_name);
if (glibc_dir_arg) |dir| {
// TODO look into making this a call to `linuxTriple`. This
// needs the directory to be called "i686" rather than
// "x86" which is why we do it manually here.
const fmt_str = "{s}" ++ fs.path.sep_str ++ "{s}-{s}-{s}";
const cpu_arch = self.target.getCpuArch();
const os_tag = self.target.getOsTag();
const abi = self.target.getAbi();
const cpu_arch_name: []const u8 = if (cpu_arch == .x86)
"i686"
else
@tagName(cpu_arch);
const full_dir = try std.fmt.allocPrint(b.allocator, fmt_str, .{
dir, cpu_arch_name, @tagName(os_tag), @tagName(abi),
});

try zig_args.append("--test-cmd");
try zig_args.append("-L");
try zig_args.append("--test-cmd");
try zig_args.append(full_dir);
}
try zig_args.append("--test-cmd-bin");
break :ok;
}
try zig_args.append("--test-no-exec");
},
.wine => |bin_name| if (b.enable_wine) {
try zig_args.append("--test-cmd");
try zig_args.append(bin_name);
try zig_args.append("--test-cmd-bin");
} else {
try zig_args.append("--test-no-exec");
},
.wasmtime => |bin_name| if (b.enable_wasmtime) {
try zig_args.append("--test-cmd");
try zig_args.append(bin_name);
try zig_args.append("--test-cmd");
try zig_args.append("--dir=.");
try zig_args.append("--test-cmd-bin");
} else {
try zig_args.append("--test-no-exec");
},
.darling => |bin_name| if (b.enable_darling) {
try zig_args.append("--test-cmd");
try zig_args.append(bin_name);
try zig_args.append("--test-cmd-bin");
} else {
try zig_args.append("--test-no-exec");
},
}
}
} else if (self.kind == .test_exe) {
try zig_args.append("--test-no-exec");
}

try self.appendModuleArgs(&zig_args);
Expand Down
5 changes: 2 additions & 3 deletions lib/std/Build/InstallArtifactStep.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@ pub fn create(owner: *std.Build, artifact: *CompileStep) *InstallArtifactStep {
.artifact = artifact,
.dest_dir = artifact.override_dest_dir orelse switch (artifact.kind) {
.obj => @panic("Cannot install a .obj build artifact."),
.@"test" => @panic("Cannot install a .test build artifact, use .test_exe instead."),
.exe, .test_exe => InstallDir{ .bin = {} },
.exe, .@"test" => InstallDir{ .bin = {} },
.lib => InstallDir{ .lib = {} },
},
.pdb_dir = if (artifact.producesPdbFile()) blk: {
if (artifact.kind == .exe or artifact.kind == .test_exe) {
if (artifact.kind == .exe or artifact.kind == .@"test") {
break :blk InstallDir{ .bin = {} };
} else {
break :blk InstallDir{ .lib = {} };
Expand Down
Loading

0 comments on commit 1b8da32

Please sign in to comment.