ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.64k stars 2.46k forks source link

testing.expectError could also accept an error #20615

Open mnemnion opened 1 month ago

mnemnion commented 1 month ago

I discovered that changing a test from something like this:

test "stuff" {
      try std.testing.expectError(error.SomeError, try throwError());      
}

To this:

fn testStuff(allocator: Allocator) !void {
     _ = throwError() catch |e| {
         switch(e) {
             error.OutOfMemory => return error.OutOfMemory,
             else => try std.testing.expectError(error.SomeError, e),
        }
    };
}

Results in a hard-to-diagnose compiler error, solved with

   try std.testing.expectEqual(error.SomeError, e);

This was while rewriting some tests to use std.testing.checkAllAllocationFailures, because of course std.testing.expectError will capture the OutOfMemory errors and then that function won't compile.

In retrospect it was clear enough that it was asking for an error union, but I was providing an error set. However, colloquially speaking, both of these are errors, and the function is named std.testing.expectError. More precisely, a single expected error can be compared as easily against an error set as against an error union.

It would be nice if the implementation did some compile-time reflection so as to also accept an error set, as well as an error union. Both of them can be equally compared against a provided expected error, and that would facilitate this sort of refactoring, as well as making std.testing.expectError conform more closely to what its name suggests it will do.

The combination of triggering all not-allocation errors, and confirming memory safety in the face of that combination, is essential for complex code with error states, so facilitating this kind of test refactor is a good thing. Also, it just makes sense for expectError to be able to handle both cases where an error needs to be confirmed: as one of a set, or as triggered at all by the test code.

mnemnion commented 1 month ago

It looks like this change would be relatively simple, and I'd be interested in making a patch which adds this if people agree that this behavior makes sense.