search

stretchr / testify

A toolkit with common assertions and mocks that plays nicely with the standard library
MIT License
23.37k stars 1.59k forks source link

Implements arg order is backwards #146

Open willfaught opened 9 years ago

willfaught commented 9 years ago

Most non-expected-actual functions, like EqualError and Len, put the object to be examined first and the expected value second, but Implements does it in reverse order: type first, then object. This makes it hard to remember.

matryer commented 9 years ago

Please fix this and submit a PR. It would be great if it were consistent.

On 19 Mar 2015, at 17:16, Will Faught notifications@github.com wrote:

Most non-expected-actual functions, like EqualError and Len, put the object to be examined first and the expected value second, but Implements does it in reverse order: type first, then object. This makes it hard to remember.

— Reply to this email directly or view it on GitHub https://github.com/stretchr/testify/issues/146.

phemmer commented 9 years ago

I would argue that Implements is correct, and Len is backwards. In most of the functions, the thing being tested against is the second argument, and the thing being tested is the third (and latter) argument(s). Thus in Len, the expected value should be the second argument.

Functions I think need to be fixed: Contains, Len, HTTPBodyContains, HTTPBodyNotContains, NotContains

willfaught commented 9 years ago

Agreed.

phemmer commented 7 years ago

Looks like another backwards method has been added since this issue was created: EqualError

mvdkleijn commented 4 years ago

I believe this issue and related PR can move forward again, but we need to take a look at #399 which makes a fair point.

willfaught commented 4 years ago

I agree with https://github.com/stretchr/testify/issues/399, actually. Go has a strong preference for the actual-then-expected order of comparison values. This ticket is just about the order—whatever it may be—being consistent among the funcs.

willfaught commented 1 year ago

Closing due to no activity.

zzh8829 commented 1 year ago

request to reopen this for v2, the v1 parameter ordering is very counter intuitive

tonyhb commented 9 months ago

+1 - Len is inconsistent. The asserted value comes last, vs EqualValues which comes first. In general, a bunch of these are inconsistent and it's pretty frustrating.

brackendawson commented 8 months ago

As this is seems to be the oldest one, and it's already in the v2.0.0 milestone, sure. If v2 ever happens the expected/actual order will be made consistent. Most likely using the order from assert.Equal as it's the most ubiquitous.

HeCorr commented 2 months ago

In regards to the naming convention, I vote for renaming the sides to "left" and "right" like they are called in Rust, eliminating the discrepancy between all the functions and making the sides unambiguous no matter which standard you're used to (expected-actual/actual-expected).

Thoughts? @willfaught @brackendawson

willfaught commented 2 months ago

That’s what I’ve done in my own test helper. Given the age of this issue, I doubt this will ever happen to testify. Better to make your own, better library.

brackendawson commented 1 month ago

Yes, left v right is interesting. But we're not going to change the order/name of any assertions in v1, even to make them consistent in the module. At this time I don't anticipate a v2.