Proposal: flip expected and actual in std.testing.expectEqual

[joachimschmidt557]

It may be useful to flip the arguments to std.testing.expectEqual. Right now, the expected value is the first formal parameter and the actual value is the second formal parameter.

When working with optional types, this makes testing via expectEqual impossible:

const std = @import("std");

pub fn expectEqual(actual: var, expected: @TypeOf(actual)) void {}

pub fn main() void {
    const x: ?u8 = 1;
    expectEqual(x, 1);

    // Doesn't work as the type of the expected value is comptime_int
    // std.testing.expectEqual(1, x);

    const y: ?u8 = null;
    expectEqual(x, null);

    // Doesn't work as the type of the expected value is (null)
    // std.testing.expectEqual(null, y);
}

But this may also be a very minor edge case.

[joachimschmidt557]

An alternative would be to create a function in std.testing to handle optional types.

[daurnimator]

Right now, the expected value is the first formal parameter and the actual value is the second formal parameter.

This is expected so that actual can be coerced to expected. It's also a common pattern for testing frameworks, see e.g. http://olivinelabs.com/busted/#assert-equals

    const y: ?u8 = null;
    expectEqual(x, null);

    // Doesn't work as the type of the expected value is (null)
    // std.testing.expectEqual(null, y);

Write: td.testing.expectEqual(@as(?u8, null), y);

[joachimschmidt557]

Ok, when other testing frameworks (Junit, NUnit, etc.) also follow these guidelines, it should be ok to just explicitly cast the expected values to the correct type.

[fengb]

I actually favor this proposal. Adding those extra coercions is pretty gross and imo negatively affects readability. So either we're stuck with the correct but clunky expectEqual(@as(?u8, null), y) or we get by with the better looking but wrong expectEqual(y, null)

Alternately, we could see about coercing the literal to the other type if possible.

[mlarouche]

I did helpers in my zigimg library to reserve the order of expected and actual because it annoyed me so much: https://github.com/mlarouche/zigimg/blob/master/tests/helpers.zig

[joachimschmidt557]

I'll reopen this issue to because there seems some support for this.

[LemonBoy]

#4438 proposes a different API that may solve this problem

[BarabasGitHub]

I just noticed I do it the wrong way around in all my test code. 😐

Because I read it like this: expect x to be 1 and not expect 1 to be x I guess.

[MageJohn]

It's not just with optional types this is annoying. For example:

const std = @import("std");

test "math" {
    const num: f64 = 1.5;
    std.testing.expectEqual(1.0, std.math.floor(num));
}

Which leads to this error:

./test.zig:5:48: error: expected type 'comptime_float', found 'f64'
    std.testing.expectEqual(1.0, std.math.floor(num));
                                               ^
./test.zig:5:28: note: referenced here
    std.testing.expectEqual(1.0, std.math.floor(num));
                           ^

As has been pointed out, this can be fixed with an explicit cast, which is ugly, or by swapping the arguments, which leads to the wrong error messages when a test fails.

Because testing is so easy to do in Zig, and because all the documentation uses tests to demonstrate concepts, new users are quite likely to use them. It may not be obvious to them that the correct solution is to cast the expected value. When I first encountered it, I "solved" the problem by swapping the arguments, which works to catch failing tests, but leads to confusing error messages.

[BarabasGitHub]

Another solution would be to make the type explicit, so it would look like:

std.testing.expectEqual(f32, 1.0, std.math.floor(num));

[alinebee]

I ended up with this wrapper as a workaround, which preserves the intended parameter order and doesn't clutter the parameter list:

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

pub fn expectEqual(expected: anytype, actual: anytype) void {
    testing.expectEqual(@as(@TypeOf(actual), expected), actual);
}

test "expectEqual correctly coerces types that std.testing.expectEqual does not" {
    const int_value: u8 = 2;
    expectEqual(2, int_value);

    const optional_value: ?u8 = null;
    expectEqual(null, optional_value);

    const Enum = enum { One, Two };
    const enum_value = Enum.One;
    expectEqual(.One, enum_value);
}

[gonzus]

FWIW, after working on several languages with good support for testing, my brain is hardwired for this:

const got = foo();
const expected = 11;
expect(got).toBe(expected);

[gwenzek]

I tried replacing the first line of expectEqual with the following:

pub fn expectEqual(expected_wrong_type: anytype, actual: anytype) !void {
    const expected = @as(@TypeOf(actual), expected_wrong_type);

It shows that actually a lot of standard library tests are using the expectEqual(actual, expected) to escape the current issue and will have to be reversed. In this commit, you can see some examples that would need to be fixed.

https://github.com/gwenzek/zig/pull/2/files

I suppose there is also a lot of unit tests in the wild using flipped arguments, so we should try to fix this ASAP. I can do the PR and fix the tests in STD lib, but I'd like to get confirmation this is the direction we want to go (since this is probably some significant amount of grunt work)

[Mouvedia]

another example: https://github.com/ziglang/zig/pull/10326#issuecomment-992006970

[nektro]

adding a new function such as expectEql and deprecating expectEqual for a time would likely be the safest way forward, otherwise all compiles would still work yet error messages would be backwards. using a different function lets users know they need to update.

also I'm willing to take this on should it be accepted

[nektro]

I've also found multiple examples of people messing up the order in the wild given how unintuitive it is

[dvmason]

There are examples in the standard library of it being backward, too. e.g. std/mem.zig assumes it in places (like test "indexOfDiff")...

The natural thing is you type the value you're expecting, and then you say... hmm.. what should it be? All the other test infrastructures I've used do the (actual,expected) too.

That is also the right way for the type propagation to go... for all the DX reasons above (comptime ints and floats, optional values). In the rare case where the actual has the type wrong, many other things are going to flag it first (e.g. probably won't compile).

I don't see any need to create a new one and deprecate the old.... no working assertion would ever even see the difference in the error. Although an alias expectEquals would be nice as it reads more smoothly.

[andrewrk]

It's pretty clear that a big flaw is that the order is difficult to remember, and it's natural to put actual first when not referencing the function signature. But putting expected first makes sense so that actual can coerce to its type.

The problem pointed out in this issue does not seem like a real problem to me:

    // Doesn't work as the type of the expected value is comptime_int
    // std.testing.expectEqual(1, x);

You have to make the expected value have the correct type:

    std.testing.expectEqual(@as(?u8, 1), x);

Shouldn't you know what type you are expecting to find?

Similarly:

    std.testing.expectEqual(@as(?u8, null), y);

This function used to take actual: anytype as well and fail the check if @TypeOf(actual) != @TypeOf(expected). But I caved and let somebody change the signature to have actual coerce to the type of expected.

As for what we should do going forward, I do think something should change because as is people, including myself, occasionally get the order backwards. However I don't think simply flipping the order the other way around is a sufficient change to address the problem.

[InKryption]

Shouldn't you know what type you are expecting to find?

In that case, why don't we just abandon the anytype here, and require a specific type?

pub fn expectEqual(comptime T: type, expected: T, actual: T) !void {...}

Edit: I notice after posting that this has been suggested before, but was not heavily considered. Is there any downside to using an explicit type here? It would seem to me that it solves most of the complaints, in particular having to write @as(T, literal).

[dvmason]

Shouldn't you know what type you are expecting to find?

As I commented, the type of the actual is almost certainly the right type... if it isn't you will quickly find out when you proceed with your implementation, because something will not type. You could say that it should be more preemptive, but Zig is already quite lazy (as opposed to eager) (which is good) about typing errors... until you need the code, mostly it just lets you proceed.

In that case, why don't we just abandon the anytype here, and require a specific type?

pub fn expectEqual(comptime T: type, expected: T, actual: T) !void {...}

Edit: I notice after posting that this has been suggested before, but was not heavily considered. Is there any downside to using an explicit type here? It would seem to me that it solves most of the complaints, in particular having to write @as(T, literal).

It's only a few character shorter than the @as, but test should be easier to write, not more difficult. I think this is a step backwards. As I said above, the actual value is the correct type (with high probability), so reversing the order solves the comptime and the optional problems, with no significant downside.

[renatoathaydes]

Not sure this adds to the conversation, but basically the first example I ever tried of using expectEqual did not compile and it's probably a little simpler than the examples above:

export fn add(a: i32, b: i32) i32 {
    return a + b;
}

test "basic add functionality" {
    try testing.expectEqual(10, add(3, 7));
}

Result:

./src/main.zig:21:36: error: expected type 'comptime_int', found 'i32'
    try testing.expectEqual(10, add(3, 7));

FTR: I agree with some of the comments that expectEqual(a,b) is kinda old fashion with the old confusion of what's the expected value VS the actual... new testing libraries should IMO try to use much more usable assertion APIs like AssertJ (if that's possible in Zig, I think it should be?)

[smokku]

The discussion here reminds me of the debate of the ordering in if comparison.

if (some_value == 5) {
  ...
}

and

if (5 == some_value) {
  ...
}

Both are equally valid code, yet one is preferable.

Could it be that people are expecting the same ordering which is preferable in if statement?

[McSinyx]

IMHO the issue is less with ordering and more with typing. In the current implementation, actual is casted to the type of expected:

pub fn expectEqual(expected: anytype, actual: @TypeOf(expected)) !void

It is a problem since expected scalars are commonly given in a comptime type or to be expected (pun unintended) to be coerced into the type of actual. This is because actual is usually given as a result of a function call and has all of the necessary typing information, which should be implicit where possible, especially at call sites. To quote the zen:

• Favor reading code over writing code.
• Reduce the amount one must remember.

[tau-dev]

Shouldn't you know what type you are expecting to find?

I don't think the types involved are relevant here; Zig has a strong type system, and the concern of the tests are that the values are correct.

Under which conditions would you need actual to be coerced to expected? For all the cases presented here, it seems like the other way around is way more useful.

I don't think simply flipping the order the other way around is a sufficient change to address the problem.

Why not?

[dvmason]

function signature. But putting expected first makes sense so that actual can coerce to its type.

Actually, actual will never be a comptime value, but expected often will be, so it's expected that we want to coerce

[matklad]

Couple of notes:

---

I think expected being first in every xUnit framework under the sun is due to original pedagogical desire to make people write specification of behavior first.

---

I personally always confuse not only the order, but semantics as well.

---

In go world, people moved from expected/actual to got/want. I find that to be a much crisper terminology.

https://github.com/golang/go/wiki/TestComments#got-before-want

---

So, I would suggest to both:

• flip the order
• switch to got/want

[IntegratedQuantum]

I think the distinction between expected and actual just adds needless complexity.

I'd suggest just naming it first and second to avoid this debate entirely.

Of course that would mean that the error output would be something like {} != {} instead of expected {}, found {}. But I don't think this is a problem because when the test fails I have to look at the code anyways to figure out how the actual and expected values were actually computed.

[codethief]

To add another data point to the discussion (and potentially help other Zig newbies like me who are googling for this topic):

I've got an ArrayList moves whose length I would like to check in a test. Unfortunately, if I use expectEqual, I get the following error:

src/generate_moves.test.zig:54:35: error: unable to resolve comptime value
    try expectEqual(6, moves.items.len);
                       ~~~~~~~~~~~^~~~

This suggests I should fix the second argument somehow and it is completely unclear how because moves.items.len not being comptime-known is the whole point of using an ArrayList. The only fix that actually works is

try expectEqual(@as(usize, 6), moves.items.len);

[cztomsik]

This suggests I should fix the second argument somehow and it is completely unclear how because moves.items.len not being comptime-known is the whole point of using an ArrayList. The only fix that actually works is

try expectEqual(@as(usize, 6), moves.items.len);

Yeah, I just got into this - I had it reversed, so I thought I'd fix it, but then I got this error, and knowing that I would need to use @as I've just left it in the wrong order.

[whatisaphone]

This caught me too.

It seems like the obvious move is to rename expectEqual's arguments but leave the types as is. Like so:

const std = @import("std");

pub fn expectEqual(actual: anytype, expected: @TypeOf(actual)) !void {
    if (actual != expected) return error.TestExpectedEqual;
}

test "expectEqual" {
    const x: ?u8 = 1;
    try expectEqual(x, 1);

    var y: ?u8 = null;
    try expectEqual(y, null);

    const num: f64 = 1.5;
    try expectEqual(std.math.floor(num), 1.0);
}

Look at that, you don't have to write out any types or casts, and every example from this thread Just Works™. What could be better?

---

And while we're bikeshedding, I've always liked that ordering better anyway. In addition to the if (value == 1) {} vs if (1 == value) {} point above, doing it backwards looks so unnatural and chaotic in practice.

expectEqual("John Quincy Adams", user.name);
expectEqual(1, user.likes);
expectEqual(Date.ymd(2023, 4, 6), user.created);

How could anyone ever prefer the above to this?

expectEqual(user.name, "John Quincy Adams");
expectEqual(user.likes, 1);
expectEqual(user.created, Date.ymd(2023, 4, 6));