add a skip reason to show up in the build runner summary

[andrewrk]

trafficstars

Extracted from https://github.com/ziglang/zig/pull/14647.

For example, a useful one will be "foreign binary", which might look like this:

[nix-shell:~/Downloads/zig/build-release]$ stage3/bin/zig build test-compiler-rt -fsummary
Build Summary: 24/35 steps succeeded; 11 skipped; 1460/1494 tests passed; 34 skipped
test-compiler-rt success
├─ run test compiler-rt-native-Debug-bare-multi-default 243 passed 6 skipped 214ms MaxRSS:15M
│  └─ zig test Debug native success 15s MaxRSS:374M
├─ run test compiler-rt-x86_64-linux-none-Debug-bare-multi-default 243 passed 6 skipped 130ms MaxRSS:15M
│  └─ zig test Debug x86_64-linux-none success 50s MaxRSS:430M
├─ run test compiler-rt-x86-linux-none-Debug-bare-multi-default 243 passed 6 skipped 229ms MaxRSS:15M
│  └─ zig test Debug x86-linux-none success 29s MaxRSS:458M
├─ run test compiler-rt-aarch64-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug aarch64-linux-none success 26s MaxRSS:412M
├─ run test compiler-rt-arm-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug arm-linux-none success 41s MaxRSS:386M
├─ run test compiler-rt-mips-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug mips-linux-none success 50s MaxRSS:414M
├─ run test compiler-rt-mipsel-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug mipsel-linux-none success 41s MaxRSS:408M
├─ run test compiler-rt-powerpc-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug powerpc-linux-none success 41s MaxRSS:404M
├─ run test compiler-rt-powerpc64le-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug powerpc64le-linux-none success 27s MaxRSS:387M
├─ run test compiler-rt-riscv64-linux-none-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug riscv64-linux-none success 36s MaxRSS:433M
├─ run test compiler-rt-x86_64-macos-none-Debug--multi-default skipped foreign binary
│  └─ zig test Debug x86_64-macos-none success 41s MaxRSS:425M
├─ run test compiler-rt-aarch64-macos-none-Debug--multi-default skipped foreign binary
│  └─ zig test Debug aarch64-macos-none success 41s MaxRSS:407M
├─ run test compiler-rt-x86-windows-msvc-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug x86-windows-msvc success 36s MaxRSS:445M
├─ run test compiler-rt-x86_64-windows-msvc-Debug-bare-multi-default skipped foreign binary
│  └─ zig test Debug x86_64-windows-msvc success 50s MaxRSS:445M
├─ run test compiler-rt-native-ReleaseFast-bare-multi-default 244 passed 5 skipped 18ms MaxRSS:15M
│  └─ zig test ReleaseFast native success 50s MaxRSS:384M
├─ run test compiler-rt-native-ReleaseSafe-bare-multi-default 243 passed 6 skipped 33ms MaxRSS:15M
│  └─ zig test ReleaseSafe native success 39s MaxRSS:326M
└─ run test compiler-rt-native-ReleaseSmall-bare-multi-default 244 passed 5 skipped 20ms MaxRSS:15M
   └─ zig test ReleaseSmall native success 40s MaxRSS:330M

[luehmann]

Maybe related: Doesn't error.SkipZigTest violate the no hidden control flow rule? For example in this example without looking at the implementation of foo you don't know that the test can be skipped.

fn foo(n: i64) !bool {
    if(n == 0) return error.SkipZigTest;
    return n > 0;
}
test {
    try std.testing.expect(try foo(42));
}

A @skipTest builtin could ensure that tests are only skipped from the test body. Coming back to the issue at hand @skipTest could also take a reason.

[matu3ba]

Doesn't error.SkipZigTest violate the no hidden control flow rule?

No. The code, which is run is in test_runner.zig and, if you want to be pedantic, it is does not include any try (at least in mainTerminal()`).

[matu3ba]

From what I understand, skipTest is decided within test/tests.zig:923

pub fn addModuleTests(b: *std.Build, options: ModuleTestOptions) *Step {
    const step = b.step(b.fmt("test-{s}", .{options.name}), options.desc);

    for (test_targets) |test_target| {
        const is_native = test_target.target.isNative() or
            (test_target.target.getOsTag() == builtin.os.tag and
            test_target.target.getCpuArch() == builtin.cpu.arch);

        if (options.skip_non_native and !is_native)
            continue; // TODO add a branch + variable to decide, if running should be skipped with message
        ...
        if (test_target.target.ofmt == std.Target.ObjectFormat.c) {
        } else {
            const run = b.addRunArtifact(these_tests);
            run.skip_foreign_checks = true;
            
            // TOOD: add optional message to skip running with, not sure if color codes like the Build summary in `./deb/bin/zig build test-compiler-rt -fsummary -Dskip-non-native=false` are necessary
            run.setName(b.fmt("run test {s}", .{qualified_name}));

            step.dependOn(&run.step);
        }
    }
    return step;

[NicoElbers]

In my mind there are 3 ways to resolve this issue. The first is to simply add more states to Step.State like skipped_oom. This however seems like a bad solution to me. ~The second solution would be to add a field to Run, something like skip_reason: []const u8 I think this is a reasonable solution but littering the struct with random fields seems like a bad idea. The third solution would be to make Step.State a tagged union, and change it to the following:~

pub const State = union(enum) {
    precheck_unstarted,
    precheck_started,
    precheck_done,
    running,
    dependency_failure,
    success,
    failure,
    /// This state indicates that the step did not complete, however, it also did not fail,
    /// and it is safe to continue executing its dependencies.
    /// It's value represents the reason for skipping
    skipped: []const u8,
    // skipped_oom is no longer needed
};

~I think the third solution is best. If people agree I will make a pull request for this solution, but before that I would like to propose my ideas if anyone sees any problems with it.~

[NicoElbers]

Upon further investigation, the third solution won't work. Step.State is cannot be a union(enum) as that doesn't work with atomics.

[NicoElbers]

I was confused between the Run in build_runner.zig and the Run in Run.zig. Making 2 of my proposed solutions simply impossible