Open zlotnika opened 6 years ago
This sometimes trips me up a bit too. I prefer "expected actual" ordering.
But Contains
isn't an expected-actual check, it's a containment check, that the "lhs contains the rhs", and in that sense, the ordering makes sense.
This makes sense to think of these tests (Equal
and Contains
) as not totally equivalent. I do, however, think they fall under the category of having 1 thing to test (a string that might be "STATUS", a map that might have an "id"), and 1 naturally hard-coded value to test for (the string "STATUS", the string "id").
The reason I'd suggest "thing to test (actual)" first would be so that Nil
, NotNil
, Empty
, etc look the same (just with a cut off argument). On the other hand, you could say the expectation defines the function being run on the actual (something like ExpectAn(t, expectation, actual) ~= ExpectAnExpectation(t, actual)
), and so in that sense it makes more sense to have the expectations, if any, come first. I don't care strongly which comes first, but I think there could be consistency in that ideal.
Now that Go 1.11 has modules and supports semver, I propose that the version is bumped to 2.0 and the API is cleaned up. It's long overdue.
Another option, because the current reliance on argument order is ambiguous, is to break up the API further into an "expect"-like API, e.g.:
// Before
assert.Equal(t, 42, answer)
assert.Contains(t, names, "bob")
// After
assert.Expect(t, answer).Equal(42)
assert.Expect(t, names).Equal("bob")
I quite like the simplicity of the existing API in that there doesn't need to be the rspec-like Expect().MoreWords()
syntax which is longer and more complex, but I agree the ordering of the parameters is confusing as they are today.
@thwarted Could you elaborate why you like the actual-expected ordering?
While I agree with your statements that there are specific differences between Equal
and Contains
that mean Contains
should have the actual on the lhs, the fact that there are some functions like Contains
where having the actual listed first is very specifically desirable raises the question why we don't have all functions have the actual listed first, since it is not as important which comes first for Equals
.
Also, we should be making it as easy as possible to transition to and from Go tests that do not use testify, so that testify is a least jarring experience. As a comparison to go tests that use only testing, it is very common for Go tests to list actual first, then expected. In the go stdlib and many libraries these are described as got
and want
, and got
almost always comes first. There are many examples in the go stdlib if you search here:
https://github.com/golang/go/search?q=got+want
https://github.com/golang/go/blob/a9dcbab/src/syscall/js/js_test.go#L44-L48 https://github.com/golang/go/blob/50bd1c4/src/math/big/decimal_test.go#L24-L26
@thwarted Could you elaborate why you like the actual-expected ordering?
I don't prefer actual-expected. In the second comment, I said «I prefer "expected actual" ordering.». To explain though, I think I prefer expected-actual order because actual is a more variable input, and I'm more comfortable with variable things at the end of argument lists (similar in idea to things like varargs always filling up the end -- obviously this isn't tantamount to "variable number of arguments", but that's some of the way I think about it). Things that are not changing, like literal values, come first, like the printf format string; and variable things and more complex (or longer) expressions come later.
assert.Equal(t, 42, 6 * 7)
assert.Equal(t, 6 * 7, 42)
I find the second one harder to read, because the more complex expression comes first. Imagine a series of these, they would be difficult to format align (if that was such a thing that gofmt didn't massage).
But while we're expressing preferences, I don't like the Expect().MoreWords()
style because it reads too much like English. I realize this is probably a lame reason, I don't have a good rationale. Chaining methods isn't a common golang thing, in my experience, and doesn't lend itself well to error handling (not that this is necessarily an issue with using this form for test assertions, it's just more popular in other languages where the focus is often to explicitly "create a DSL". Does go need a DSL for tests? Would that be better?).
I like that this testify package makes writing tests easier and shorter. That would go away with more verbosity. I've got an in-house module that uses jsonpath to provide similar succinctness for doing JSON structure/value testing (we were using assert.Contains
on strings for that, which isn't very robust), and I did pause for a bit when writing it to see if the argument ordering for that use case could be made "consistent" with other uses.
I'm probably going to have to continue to have the documentation open anyway. I don't consider this to be a hindrance. I think there is some inconsistency, but I also think my earlier observation (But Contains isn't an expected-actual check
) stands. I think of this the same way some people think the ordering of arguments to memcpy/memmove are confusing because it doesn't match other APIs that order src-dest (aka from-to) ordering: the ordering of memcpy arguments match the ordering of arguments of the assignment operator (memcpy(dest, src, …)
== dest = src
). The memcpy argument ordering is consistent with what it is meant to support that C doesn't have directly (using the assignment operator on memory ranges).
@thwarted I apologise, that was a typo on my part. I meant to ask, could you elaborate why you prefer expected-actual
.
Either way, thanks for elaborating.
I like that this testify package makes writing tests easier and shorter.
💯 This is especially testify's strength. There are other packages like Ginkgo that offer support for rspec-like DSL Expect(...).To(Equal(...))
and for those packages that is their strength.
One reason I find actual-expected
natural is it fits left-to-right in my brain with how I read expressions. I say to myself 6 x 7 should be 42
. I write if statements the same way, if x == 42
. I write sql the same way, where x = 42
.
Another reason I find actual-expected
natural is so many other testing libraries/frameworks I've used are actual-expected.
actual-expected
expected-actual
These are just ones I've come across. I'm going to guess there are more frameworks in both camps. I find it interesting that so many of the Go ones exhibit actual-expected
, and I think there's value in being consistent with them.
Perl's Test::More
also uses actual-expected
(aka got-want
)
I personally prefer the actual, expected
ordering which makes more sense, because that's how we write if statements:
number == 3
("number equals three"), not 3 == number
("three equals number").
The latter just feels very counter-intuitive and hard to understand quickly in my opinion.
I've just been staring at a failed assert message wondering why it was expecting the wrong value and noticed that Equal()
's parameter order was the opposite of what I thought it was.
Was there a specific reason for this design choice?
I don't personally have any strong preference whether it's actual-expected
or expected-actual
. I just find it really irritating that I make a lot of mistakes writing tests when switching between Equal()
, Contains()
, Len()
, ErrorIs()
etc. The constant juggling between expected
and actual
is just tedious. I pretty often do assertions like presented below:
// 1. expected 3 results
assert.Len(t, results, 3)
// 2.1. then I copy-paste the previous line
assert.Equal(t, results, 1)
// 2.2. then change it to my expected results
assert.Equal(t, results[0], "text_1")
// 2.3. then swap the parameter order
assert.Equal(t, "text_1", results[0])
It's just one of those little things that makes working with this library annoying.
I really like how Rust does it, calling them "left" and "right" which is very unambiguous, no matter what convention you're used to.
While the terms "left" and "right" match the positioning in a comparison expression and function arguments, it doesn't communicate which value is the one that is expected and which one is being tested, which has both presentation-of-the-failure concerns and debugging concerns, which I think is really the issue. It's not enough that a comparison failed, what the target state is and how the result differs from the target state is what makes a good test failure.
Covered by testifylint#expected-actual
I have noticed that the parameter ordering seems inconsistent to what I would expect. An example of this:
Notice that in
Equal
, we give first the expected value, then the actual value whereas inContains
, we give first the actual map, then the expected value. I'd recommend always doing test struct, then the thing you're testing, then expected value (if it's a comparison).