Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specializations of assertEquals() and assertNotEquals() #3340

Closed
sebastianbergmann opened this Issue Oct 11, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Oct 11, 2018

Implement the following methods

  • assertEqualsCanonicalizing()
  • assertEqualsIgnoringCase()
  • assertEqualsWithDelta()
  • assertNotEqualsCanonicalizing()
  • assertNotEqualsIgnoringCase()
  • assertNotEqualsWithDelta()

to replace the respective optional parameters for assertEquals() and assertNotEquals().

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Oct 11, 2018

What about combinations of this flags:

  • asserting that 2 arrays contains the same case-insensitive values without a constraint on the order (canonicalize + ignore case)
  • asserting that 2 arrays contains the same floats (with delta) without a constraint on the order (canonicalize + delta)

The new methods don't account for all combinations of it (and the comparator library still has them as optional parameters AFAICT, so these combinations would still be supported by the lower-level code, unless the goal is to change these APIs too)

@sebastianbergmann

This comment has been minimized.

Copy link
Owner Author

sebastianbergmann commented Oct 11, 2018

I see your point, @stof. However, the combinatorial explosion made possible by mixing the various optional parameters of assertEquals() lead at least to confusion in the past. See #3317, for example. I would really like to reduce the complexity and confusion here. I am also not convinced that there are (enough) real use cases for the combinations to justify them.

I could see, however, keeping the $delta argument (somehow it feels "special") for assertEquals() and assertNotEquals() (removing the need for assertEqualsWithDelta() and assertNotEqualsWithDelta()) as well as adding $delta as an optional parameter to assertEqualsCanonicalizing() and assertNotEqualsCanonicalizing().

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Oct 11, 2018

#3317 is not about assertEquals but about assertContains. AFAICT, ignoreCase should be working fine for arrays (in case they contain strings of course, otherwise there is no point for ignoring the case).

@stof

This comment has been minimized.

Copy link
Contributor

stof commented Oct 11, 2018

note however that I won't fight strongly for keeping the combination of canonicalize and ignoreCase, as I don't remember when I used ignoreCase in my own tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.