search

stretchr / testify

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

assert: new assertion NotElementsMatch #1600

Closed hendrywiranto closed 6 months ago

hendrywiranto commented 6 months ago

Summary

Add new assertion NotElementsMatch which asserts whether 2 lists are NOT equal ignoring the order of the elements. If there are duplicate elements, the number of appearances of each of them in both lists should not match. This is an inverse of ElementsMatch.

Changes

Motivation

Related issues

https://github.com/stretchr/testify/issues/1594 https://github.com/stretchr/testify/discussions/1592

brackendawson commented 6 months ago

I had read the user's request as wanting NotElementsMatch (inverse of ElementsMatch) rather than NoElementsMatch, which a new and different assertion.

NotElementsMatch is an easy sell because by not having a negative it's inconsistent with the rest of the API and because a user requested it.

NoElementsMatch makes sense but is harder to justify because it's a distinct assertion and because nobody asked for it. Does it have an opposite? NotNoElementsMatch? OneElementMatches?

hendrywiranto commented 6 months ago

I had read the user's request as wanting NotElementsMatch (inverse of ElementsMatch) rather than NoElementsMatch, which a new and different assertion.

NotElementsMatch is an easy sell because by not having a negative it's inconsistent with the rest of the API and because a user requested it.

NoElementsMatch makes sense but is harder to justify because it's a distinct assertion and because nobody asked for it. Does it have an opposite? NotNoElementsMatch? OneElementMatches?

Hello, thanks for the review I agree with you, should've just made the NotElementsMatch I never intend to make a new assertion, it's a misunderstanding between the two assertion, I was thinking those two would be the same I'll update the PR

hendrywiranto commented 6 months ago

I adjusted the PR to NotElementsMatch, please kindly review again @brackendawson thank you 😄

hendrywiranto commented 6 months ago

Looks great. I considered if ElementsMatch should be moved to a private function and wrapped by ElementsMatch and NotElementsMatch, but this is already done with diffLists. 👍

and the diffLists has already been tested separately too so we can use it like this 💯

Would like to see the references to the lists in the failure messages match the parameter names, though.

Thanks for the input, adjusted!