MF-171: Replace argThat with eager failures #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stoyicker
force-pushed
the
jantonio/combined_pass_allof
branch
3 times, most recently
from
36848eb
to
ee21d98
Compare
stoyicker
force-pushed
the
jantonio/playlog_test_20
branch
from
ab583f4
to
1d9ad7f
Compare
stoyicker
force-pushed
the
jantonio/combined_pass_allof
branch
from
ee21d98
to
0d7b2c0
Compare
argThat is not descriptive enough for composite conditions.
stoyicker
force-pushed
the
jantonio/combined_pass_allof
branch
from
0d7b2c0
to
5cf2923
Compare
stoyicker
changed the title
stoyicker
force-pushed
the
jantonio/playlog_test_20
branch
from
1d9ad7f
to
2572834
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
argThat is not descriptive enough for composite conditions.
Long story:
As you may have noticed, the tests that use a single verify call use argThat to look at the payload. If a test passes this works well and if it fails it's straightforward to figure out where the issue is - there's only one call and because we use assertions inside argThat we'll know exactly what failed, instead of returning a boolean and causing a situation where argThat would just print the whole String for every call, which has bonus problems for us since we only really care about the calls that are done for the playback_session name, but there are more than those for the same method and all of them will be printed out.
But for tests with more than one PlayLog event verification this approach does not work: we cannot use assertions inside argThat because that will prevent verifications from passing when written in an order different to that in which the corresponding calls are verified, and since we cannot guaranteed that order, relying on it is a no-no. We could use argThat as intended instead of with assertions and just return a boolean though, that works - but if the test fails, it becomes really hard to figure out which call failed and why.
This PR introduces a mechanism to tackle this issue.