-
Notifications
You must be signed in to change notification settings - Fork 613
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
Replace usages of @Test(expected) with Assert.assertThrows(). #1579
Replace usages of @Test(expected) with Assert.assertThrows(). #1579
Conversation
iterator.next(); | ||
} | ||
assertThrows(NoSuchElementException.class, () -> iterator.next()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These overrides don't make sense - I'll remove them. The test is for non-empty-collection, and this overrides it to only make assertions about empty collections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggested change inline in regards to missing static import.
public void get_throws_index_greater_than_size() | ||
{ | ||
this.classUnderTest().get(this.classUnderTest().size()); | ||
Assert.assertThrows(IndexOutOfBoundsException.class, () -> this.classUnderTest().get(this.classUnderTest().size())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you not want to make this a static import since you are changing this anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sure, it was like that in the original but I'll change it.
f877789
to
acd17e5
Compare
} | ||
iterator.next(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test is almost identical to the superclass, but it uses add() instead of a varargs creation method. Calling add() is the reason it doesn't work in unmodifiable tests which results in even more overrides.
acd17e5
to
b9b786a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Relates to #1578 and #1576. It turns out that just moving away from JUnit 4 syntax for expected exceptions is a fairly large change on its own. @Desislav-Petrov please take a look.