Skip to content

Commit

Permalink
test_runner: enable testing panics in mainTerminal
Browse files Browse the repository at this point in the history
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
  • Loading branch information
matu3ba committed May 8, 2023
1 parent 16314e0 commit f76e02a
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 7 deletions.
10 changes: 10 additions & 0 deletions TODO
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
- [x] parse `--mustpanic`
- [x] design works with multithreading
- [ ] parse panic message
- [ ] server integration
- [ ] move into function
- [ ] clarify how the server logic should work
* must store remaining time left for process,
* must store gid+pid to kill it, if taking too long
* must regularly check, if already finished
* double check where list of unfinished tests and status is stored
7 changes: 5 additions & 2 deletions lib/std/child_process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ pub const ChildProcess = struct {
}

/// Blocks until child process terminates and then cleans up all resources.
/// In case of error, the caller is responsible to clean up the ressources
/// via calling `self.cleanupStreams()`.
/// TODO: This describes the current state. Is this intended?
pub fn wait(self: *ChildProcess) !Term {
const term = if (builtin.os.tag == .windows)
try self.waitWindows()
Expand Down Expand Up @@ -312,7 +315,7 @@ pub const ChildProcess = struct {
};

/// Spawns a child process, waits for it, collecting stdout and stderr, and then returns.
/// If it succeeds, the caller owns result.stdout and result.stderr memory.
/// If spawning succeeds, then the caller owns result.stdout and result.stderr memory.
pub fn exec(args: struct {
allocator: mem.Allocator,
argv: []const []const u8,
Expand Down Expand Up @@ -415,7 +418,7 @@ pub const ChildProcess = struct {
self.term = self.cleanupAfterWait(status);
}

fn cleanupStreams(self: *ChildProcess) void {
pub fn cleanupStreams(self: *ChildProcess) void {
if (self.stdin) |*stdin| {
stdin.close();
self.stdin = null;
Expand Down
3 changes: 3 additions & 0 deletions lib/std/std.zig
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ pub const options = struct {
options_override.side_channels_mitigations
else
crypto.default_side_channels_mitigations;

/// Default thread-local storage panic message size used for panic tests.
pub const testing_max_panic_msg_size = 100;
};

// This forces the start.zig file to be imported, and the comptime logic inside that
Expand Down
29 changes: 29 additions & 0 deletions lib/std/testing.zig
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,35 @@ test "expectEqualDeep composite type" {
}
}

const can_panic_test = builtin.is_test and !builtin.single_threaded and std.process.can_spawn;

/// Static storage to support user-generated panic messages
/// Parent process writes these, returns to the test execution loop and spawns,
/// child process ignores these.
const TestFn_iT = if (can_panic_test) ?[std.options.testing_max_panic_msg_size]u8 else void;
pub threadlocal var panic_msg: TestFn_iT = if (can_panic_test) null else {};

/// Distinguishes between parent and child, if panics are tested for.
/// TODO: is_panic_parentproc and panic_msg feels like it belongs into test api to
/// allow implementations providing their own way to prevent the necessity to use tls.
var is_panic_parentproc: if (can_panic_test) bool else void = if (can_panic_test) true else {};

/// To be used for panic tests after test block declaration.
pub fn spawnExpectPanic(msg: []const u8) error{ SpawnZigTest, SkipZigTest }!void {
std.debug.assert(can_panic_test); // Caller is responsible to check.
if (is_panic_parentproc) {
if (panic_msg == null) {
@memcpy(panic_msg, msg); // Message must be persistent, not stack-local.
return error.SpawnZigTest; // Test will be run in separate process
} else {
@panic("std.testing.panic_msg must be only used in spawnExpectPanic");
}
} else {
std.debug.assert(panic_msg == null);
// panic runner continues running the test block
}
}

fn printIndicatorLine(source: []const u8, indicator_index: usize) void {
const line_begin_index = if (std.mem.lastIndexOfScalar(u8, source[0..indicator_index], '\n')) |line_begin|
line_begin + 1
Expand Down
2 changes: 1 addition & 1 deletion lib/std/zig/Server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub const Message = struct {
/// - 0 means not async
/// * expected_panic_msg: [tests_len]u32,
/// - null-terminated string_bytes index
/// - 0 means does not expect pani
/// - 0 means does not expect panic
/// * string_bytes: [string_bytes_len]u8,
pub const TestMetadata = extern struct {
string_bytes_len: u32,
Expand Down
59 changes: 55 additions & 4 deletions lib/test_runner.zig
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,22 @@ pub fn main() void {
for (args[1..]) |arg| {
if (std.mem.eql(u8, arg, "--listen=-")) {
listen = true;
} else if (std.mem.eql(u8, arg, "--mustpanic")) {
std.testing.is_panic_parentproc = false;
} else {
@panic("unrecognized command line argument");
}
}

if (listen) {
return mainServer() catch @panic("internal test runner failure");
return mainServer(args) catch @panic("internal test runner failure");
} else {
return mainTerminal();
return mainTerminal(args);
}
}

fn mainServer() !void {
fn mainServer(args: []const u8) !void {
_ = args;
var server = try std.zig.Server.init(.{
.gpa = fba.allocator(),
.in = std.io.getStdIn(),
Expand Down Expand Up @@ -126,7 +129,9 @@ fn mainServer() !void {
}
}

fn mainTerminal() void {
fn mainTerminal(args: []const u8) void {
// TODO make environment buffer size configurable and use a sane default
// Tradeoff: waste stack space or allocate on every panic test
const test_fn_list = builtin.test_functions;
var ok_count: usize = 0;
var skip_count: usize = 0;
Expand Down Expand Up @@ -185,6 +190,52 @@ fn mainTerminal() void {
progress.log("SKIP\n", .{});
test_node.end();
},
error.SpawnZigTest => {
if (!std.testing.can_panic_test)
@panic("Found error.SpawnZigTest without panic test capabilities.");
if (std.testing.panic_msg == null)
@panic("Panic test expects `panic_msg` to be set. Use std.testing.spawnExpectPanic().");

var child_proc = std.ChildProcess.init(
&.{ args[0], "--mustpanic" },
std.testing.allocator,
);

child_proc.stdin_behavior = .Ignore;
child_proc.stdout_behavior = .Pipe;
child_proc.stderr_behavior = .Pipe;
child_proc.spawn() catch |spawn_err| {
progress.log("FAIL ({s})\n", .{@errorName(spawn_err)});
continue;
};

var stdout = std.ArrayList(u8).init(std.testing.allocator);
defer stdout.deinit();
var stderr = std.ArrayList(u8).init(std.testing.allocator);
defer stderr.deinit();
try child_proc.collectOutput(&stdout, &stderr, args.max_output_bytes) catch |collect_err| {
progress.log("FAIL ({s})\n", .{@errorName(collect_err)});
continue;
};
const term = child_proc.wait() catch |wait_err| {
child_proc.cleanupStreams();
progress.log("FAIL wait_error (exit_status: {d})\n, stdout: ({s})\n, stderr: ({s})\n", .{ @errorName(wait_err), stdout.items(), stderr.items() });
continue;
};
switch (term) {
.Exited => |code| {
progress.log("FAIL (exit_status: {d})\n, stdout: ({s})\n, stderr: ({s})\n", .{ code, stdout.items(), stderr.items() });
continue;
},
.Signal => |code| {
_ = code;
// TODO
// - check that we have panicked
// - parse panic msg
//
},
}
},
else => {
fail_count += 1;
progress.log("FAIL ({s})\n", .{@errorName(err)});
Expand Down
23 changes: 23 additions & 0 deletions testfile.zig
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! minimal test file

const std = @import("std");
const testing = std.testing;

test "panic" {
testing.spawnExpectPanic("test1");
@panic("test1");
}

test "wrong_panic" {
testing.spawnExpectPanic("test2");
@panic("test1");
}

test "no panic but one was expected" {
testing.spawnExpectPanic("test3");
}

// unhandled case:
// test "panic but none was expected" {
// @panic("test4");
// }

0 comments on commit f76e02a

Please sign in to comment.