Skip to content
New issue

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

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

Already on GitHub? # to your account

Clean Unit test up (favor ExpectedException over try catch fail) #17

Merged
merged 1 commit into from
Dec 16, 2015
Merged

Clean Unit test up (favor ExpectedException over try catch fail) #17

merged 1 commit into from
Dec 16, 2015

Conversation

vanniktech
Copy link
Contributor

No description provided.


public class PrivateConstructionCheckerTest {
@Rule public ExpectedException expectedException = ExpectedException.none();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good approach

@nikitin-da
Copy link
Collaborator

LGTM @artem-zinnatullin PTAL, please

expectedException.expectMessage("expectedTypeOfException can not be null");
PrivateConstructorChecker
.forClass(Object.class)
.expectedTypeOfException(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please divide forClass and expectedTypeOfException, I mean, to make sure that it fails after call to the expectedTypeOfException and not to the forClass

@artem-zinnatullin
Copy link
Member

Sorry for tons of same comments, but they'll help during next iteration of code review

Reordered ExpectedException to be closer to the origin
@vanniktech
Copy link
Contributor Author

@artem-zinnatullin don't worry. You're right. I hope I got every place.

@artem-zinnatullin
Copy link
Member

👍

artem-zinnatullin added a commit that referenced this pull request Dec 16, 2015
Clean Unit test up (favor ExpectedException over try catch fail)
@artem-zinnatullin artem-zinnatullin merged commit 55210e7 into pushtorefresh:master Dec 16, 2015
@artem-zinnatullin
Copy link
Member

@vanniktech thanks!

@vanniktech vanniktech deleted the master_unit_test_cleanup branch December 16, 2015 13:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants