Zig build system does not fail when a test fails

[Hejsil]

Zig Version

0.11.0-dev.2165+8f481dfc3

Steps to Reproduce and Observed Behavior

// build.zig
const std = @import("std");

pub fn build(b: *std.Build) void {
    const exe_tests = b.addTest(.{
        .root_source_file = .{ .path = "src/main.zig" },
    });
    const test_step = b.step("test", "Run unit tests");
    test_step.dependOn(&exe_tests.step);
}

// src/main.zig
test "simple test" {
    unreachable;
}

$ zig build test

Expected Behavior

I expect to get a stack trace in my terminal showing the test failure. Instead, the command silently exits with a successful status code.

Running zig build test --verbose we can see that it seems to be actually running the test:

$ zig build test --verbose
<path-to>/zig test <path-to>/src/main.zig --cache-dir <path-to>/zig-cache --global-cache-dir <path-to>/.cache/zig --name test --enable-cache --listen=-

If I take the same command, remove --listen=-, this works just fine:

$ <path-to>/zig test <path-to>/src/main.zig --cache-dir <path-to>/zig-cache --global-cache-dir <path-to>/.cache/zig --name test --enable-cache
<path-to>/zig-cache/o/25622b855bc097280dff3f8cb897732b
Test [1/1] test.simple test... thread 80083 panic: reached unreachable code
<path-to>/src/main.zig:2:5: 0x20dc95 in test.simple test (test)
    unreachable;
    ^
...

[scheibo]

After #14647:

There is no difference between test and test_exe anymore, so the latter is gone. You have to add a run step for test execution now - that's a breaking change

[Hejsil]

After #14647:

There is no difference between test and test_exe anymore, so the latter is gone. You have to add a run step for test execution now - that's a breaking change

That does seem to be the issue. Seems you now need:

test_step.dependOn(&exe_tests.run().step);

Would be nice if zig init-exe had this as I turned to that command to do a small reproduction :)

[scheibo]

Would be nice if zig init-exe had this as I turned to that command to do a small reproduction :)

Agreed! And I'm sure this will be called out in the release notes when theyre written, because I also found this to be a rather nefarious breaking change (a good breaking change would break everyones tests. this one sadly makes them all pass!)

[andrewrk]

I agree it is nefarious - perhaps addTest could be renamed to addTestExecutable and the previous name could be a compile error that explains the new behavior.

[nurpax]

I'm trying to get my tests to run after upgrading to Zig 0.11.0-dev.2892+fd6200eda but I just can't get the tests to actually run. Just like OP, if I take the last test execution command from zig build test --verbose and remove the -listen=- flag, tests run just fine. I thought I'm doing everything below that's been discussed in this thread:

    const test_exe = b.addTest(
        .{
            .target = target,
            .optimize = optimize,
            .root_source_file = .{ .path = "src/main.zig" },
        },
    );
    addMainDeps(b, test_exe, target, optimize);
    test_exe.addCSourceFile("src/sgtesting.c", &.{});
    b.installArtifact(test_exe);
    const run_test = b.addRunArtifact(test_exe);
    if (b.args) |args| {
        run_test.addArgs(args);
    }
    const test_step = b.step("test", "Run tests");
    test_step.dependOn(&run_test.step);

[leecannon]

@nurpax if you run zig build test -fsummary what does it show?

If the run step shows as cached it means the code hasn't changed so the tests won't run again, but the summary will include the number of passed test cases from the prior execution.

To force the tests to always run you would need to add run_test.has_side_effects = true;.

[nurpax]

C:\Users\janne\dev\platformerzig>zig build -fsummary test
Build Summary: 3/3 steps succeeded
test cached
+- run test cached
   +- zig test Debug native cached 1s MaxRSS:22M

If I run the test.exe from the zig-cache dir, here's what it prints:

All 12 tests passed.

Adding run_test.has_side_effetcs=true, I see:

Build Summary: 3/3 steps succeeded; 12/12 tests passed
test success
+- run test 12 passed 246ms MaxRSS:25M
   +- zig test Debug native cached 1s MaxRSS:22M

Without -fsummary, it prints nothing. It's a little confusing.

[perillo]

@Vexu: wasn't this fixed in 406706f?

[Vexu]

Yes, but it could still be mitigated like #15009 (comment) suggests.

I'll leave it to Andrew to decide.

[andrewrk]

I apologize for handling this upgrade poorly. I should have done it in a strategy that caused compile errors in the build script rather than changing the behavior silently. However, I think the opportunity to handle the upgrade more smoothly has passed, and so I am going to merely improve the doc comments for addTest and then move on.