Getting `zig build test` to find all the tests in my module

[kj4tmp]

Edit: Changed title from “Should I put std.testing.refAllDecls(@This()); in all my files?”

Ziggits docs suggest using

test {
    std.testing.refAllDeclsRecursive(@This());
}

in my root.zig

Build System Tricks

This community-driven doc is an attempt to expand on the official Zig Build System documentation as well as the past community contributions by xq, Sobeston and others. Here you’ll find a list of tricks on how to use every part of the build system while making sure to conveniently name and lay out your build steps. I. Basic Usage 1) Declare commonly used information upfront // Resolve a query for a custom target const my_target = b.resolveTargetQuery(.{ .cpu_arch = .wasm32, .os_tag =…

But this is problematic when I declare a top-level declaration like so:

pub const SyncM = packed struct(u64) {
    physical_start_address: u16,
    length: u16,

    /// control register
    control: esc.SyncManagerControlRegister,
    status: esc.SyncManagerActivateRegister,
    enable_sync_manager: EnableSyncMangager,
    syncM_type: SyncMType,
};
pub const SMCatagory = std.BoundedArray(SyncM, max_sm);

Because this will reference the Writer of a bounded array, which is only valid if the bounded array is full of u8.

zig_install/lib/std/bounded_array.zig:282:13: error: The Writer interface is only defined for BoundedArray(u8, ...) but the given type is BoundedArray(sii.SyncM, ...)
            @compileError("The Writer interface is only defined for BoundedArray(u8, ...) " ++

My first inclination is to put

test {
    std.testing.refAllDecls(@This());
}

at the bottom of all my files, which, after doing it, has found multiple errors in some of my function declarations…which is nice.

[kj4tmp]

The standard library appears to use a mix of

test {
    std.testing.refAllDecls(Cpu.Arch);
}

and manual references to each sub-module

test {
    _ = AnyReader;
    _ = AnyWriter;
    _ = @import("io/bit_reader.zig");
    _ = @import("io/bit_writer.zig");
    _ = @import("io/buffered_atomic_file.zig");
    _ = @import("io/buffered_reader.zig");
    _ = @import("io/buffered_writer.zig");
    _ = @import("io/c_writer.zig");
    _ = @import("io/counting_writer.zig");
    _ = @import("io/counting_reader.zig");
    _ = @import("io/fixed_buffer_stream.zig");
    _ = @import("io/seekable_stream.zig");
    _ = @import("io/stream_source.zig");
    _ = @import("io/test.zig");
}

[andrewrk]

No. I will be very sad if that function survives to 1.0.

My most recent idea to solve this was introduce `reachable` enum tag to `std.builtin.BranchHint`; all functions reachable by default · Issue #21511 · ziglang/zig · GitHub but it was a seriously flawed idea.

I haven’t given up yet though.

In the meantime, the best thing you can do is try to test your code as much as possible, and for those functions too annoying to test, use _ = foo; on them to at least type check them.

Another idea I just thought of is to allow this syntax:

test foo;

which would be equivalent to:

test {
    _ = &foo;
}

This at least looks like a doctest but without the actual work of writing a doctest.

[dimdin]

I am using:

comptime {
    std.testing.refAllDecls(@This());
}

I am using comptime instead of test because I don’t want to count it as an additional test case. (refAllDecls uses builtin.is_test to return immediately if the code is not running under a test runner).

I don’t use refAllDeclsRecursive because I don’t write deeply nested test cases.

[kj4tmp]

This is mostly about how to get

zig build test

to find and run all of the tests in all my files.

And less about trying to get the compiler to detect dead code.

[andrewrk]

It’s the same problem

[kj4tmp]

couldn’t resist

[dimdin]

test foo;
where foo is a namespace; that means that the following may also be valid:

test @This();

or

const Self = @This();
test Self;

[kj4tmp]

Maybe something like

std.testing.refAllTestsRecursive

?

But this would likely have the unfortunate side-effect of running tests in the standard library?

Well actually as long I as dont pub my std it would be fine?

const std = @import("std");

Well it would probably still run the std.BoundedArray tests from my example if they exist.

[Sze]

@kj4tmp

Because this will reference the Writer of a bounded array, which is only valid if the bounded array is full of u8.

I don’t understand which code actually contains the access to BoundedArray(sii.SyncM, ...).Writer

couldn’t you just not access .Writer unless it is actually BoundedArray(u8, ...)?

[kj4tmp]

consider the following example:

const std = @import("std");

// a sub module of this module, that may or may not have test declarations in it.
pub const eni = @import("eni.zig");
/// now imagine about 20 more sub modules...
pub const MyStruct = struct {
    a: u8,
    b: u8,
};

pub const MyStructArray = std.BoundedArray(MyStruct, 32);

test {
    std.testing.refAllDeclsRecursive(@This());
}

when run with zig test the following compile error is emitted:

error: The Writer interface is only defined for BoundedArray(u8, ...) but the given type is BoundedArray(test.MyStruct, ...)
            @compileError("The Writer interface is only defined for BoundedArray(u8, ...) " ++

This is because std.testing.refAllDeclsRecursive(@This());
reaches into MyStructArray and does this:

_ = MyStructArray.Writer

[kj4tmp]

So the leading solution (in use by the standard library)

is to do something like this, which isnt too bad

const std = @import("std");

// a sub module of this module, that may or may not have test declarations in it.
pub const eni = @import("eni.zig");
/// now imagine about 20 more sub modules...
pub const MyStruct = struct {
    a: u8,
    b: u8,
};

pub const MyStructArray = std.BoundedArray(MyStruct, 32);

test {
    _ = @import("eni.zig");
    // 20 more of these...
}

or use the non-recursive version and put it at the bottom of all my files

test {
    std.testing.refAllDecls(@This());
}

[Sze]

Ahh okay, I didn’t think about refAllDeclsRecursive itself causing the reference.

Yeah we had a topic about this problem of not being able to avoid this error from comptime code that may not know that accessing a decl may trigger a @compileError here:

A not-immediately-fatal way of indicating that a feature is unavailable

In many places in the standard library, you see the use of @compileError() as a mean to indicate that a feature is unavailable for one reason or another. Sometimes it’s a function: pub const min = @compileError("deprecated; use @min instead"); Sometimes it’s a type: pub const Writer = if (T != u8) @compileError("The Writer interface is only defined for ArrayList(u8) " ++ "but the given type is ArrayList(" ++ @typeName(T) ++ ")") else std…

Some part of me thought that it would be nice if @compileError would just return a compileError-value that encapsulates the error and line information of the error, and then you could use comptime to inspect that error without it actually being thrown as an error. Only when this value gets compiled into the program in some way it would actually be triggered as an error.

Basically it would evaluate to a comptime value which then needs to disappear, but if it is referenced by / compiled into runtime code than it is triggered as error.
The value also could be opaque if the comptime code should not be able to access to much of the error message.

But I don’t understand the compiler enough, to say if that would work.
If the comptime type info was just enough to tell that the value resolves to a compile error, then refAllDecls could avoid referencing such declarations.

But from what Andrew wrote above, the refAllDecls is supposed to become unnecessary anyway, still being able to avoid @compileErrors could be useful for other code…

[mnemnion]

@Sze

Some part of me thought that it would be nice if @compileError would just return a compileError-value that encapsulates the error and line information of the error, and then you could use comptime to inspect that error without it actually being thrown as an error. Only when this value gets compiled into the program in some way it would actually be triggered as an error.

If you look at the fields of StructField next to the field (singular) of Declaration you’ll see a way out of this problem: add a .type field to Declaration. There are some issues discussing this, as well as having a compile_error type as a comptime equivalent of noreturn.

There are some difficulties with doing it that way without it forcing eager evaluation, but we need some sort of way to introspect declarations without halting the compilation process. It could be @TypeOfDecl(thing, "name"), I don’t mind that at all, as long as it solves the exploding reference problem.

That way std.testing.refAllDeclsRecursive could look like this:

pub fn refAllDeclsRecursive(comptime T: type) void {
    if (!builtin.is_test) return;
    inline for (comptime std.meta.declarations(T)) |decl| {
        const DeclT = @TypeOfDecl(T, decl.name);
        if (DeclT == type) {
            switch (@typeInfo(@field(T, decl.name))) {
                .@"struct", .@"enum", .@"union", .@"opaque" => refAllDeclsRecursive(@field(T, decl.name)),
                else => {},
            }
        }
        if (DeclT != compile_error) {
            _ = &@field(T, decl.name);
        }
    }
}

Although the last bit probably isn’t needed because to get the type of a declaration it has be be analyzed.

I don’t actually care whether the function stays or goes, just about having a way to introspect declarations without causing a compile error. It’s a classic use/mention mismatch: we want some kind of way of saying “analyze this, but this function call will not put the type into use in the program, so just tell us if it won’t compile, don’t halt”.