-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
rename expectThrows() to assertThrows() #1504
rename expectThrows() to assertThrows() #1504
Conversation
src/main/java/org/junit/Assert.java
Outdated
* @return the exception thrown by {@code runnable} | ||
* @since 4.13 | ||
*/ | ||
public static <T extends Throwable> T expectThrows(Class<T> expectedThrowable, ThrowingRunnable runnable) { | ||
public static <T extends Throwable> T assertThrows(Class<T> expectedThrowable, | ||
ThrowingRunnable runnable, String message) { |
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.
In JUnit 4.x the message comes first in the assertion methods, so perhaps we should do that to be consistent.
Besides consistency, the lambda have often be multiple lines, so I personally find it easier to read if the lambda is the last argument
@rschmitt Now that JUnit5 is out and stable, we wanted to (belatedly) prepare to release 4.13. Since JUnit5 combined |
@kcooney I think that merging them is fine; it's better to be consistent with JUnit 5. |
src/main/java/org/junit/Assert.java
Outdated
} | ||
|
||
private static String buildPrefix(String message) { | ||
return message != null && message.length() != 0 ? message + " ==> " : ""; |
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.
Can you please replace ==>
with something unobstrusive like a colon :
. Thanks.
@panchenko Do you have time to squash the commits? |
I can, also can be done while merging. |
I would like to squash and then rebase. Unfortunately this is not supported by GitHub's web UI. |
8d16892
to
f1972d7
Compare
Merged. Thanks, @panchenko. |
I changed the release notes accordingly. |
as suggested again in #1496, Hopefully it helps with having a release sooner.
The important part is making a release, it's not a big deal if there is 1 or 2 methods.