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

Removed assertion roulette (test smell) #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eas5
Copy link

@eas5 eas5 commented Jul 27, 2020

An Assertion Roulette occurs when a test method has multiple non-documented assertions. Various assertion statements in a test method without a descriptive message impacts readability/understandability/maintainability as it is difficult to understand the reason for the failure of the test.

In the test, I replaced the first assertions by assumptions, which verify the test preparation but will skip its execution (thus not failing the test) if not met.

The series of assertions that compared the messages retrieved from the DB with the List created in the test setup can be replaced by a single assertion comparing both collections since both are ArrayLists with the same message order.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 75.944% when pulling 0e9ec31 on eas5:test_improvement into 995eb30 on jfaster:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants