-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
assertTrue()/assertFalse() should include an error message by default #1030
Comments
Would such default error messages be helpful? You'd have to look at the test code to figure out what went wrong anyway, wouldn't you? BTW Please use |
Let's say I write this test: User user = ...; The test fails with an AssertionError with no message. There's no way to tell without inspecting the code and inspecting line numbers which assertion failed. Why is more data a bad thing? |
More data is not a bad thing. In this case, I can only see a tiny bit of more data, though. While I'm not against this feature request, I think there a better ways to formulate assertions nowadays, e.g. Hamcrest or Truth. Nevertheless, I think if someone would submit a pull request for this issue, we would probably merge it... ;-) @junit-team/junit-committers Any objections? |
I certainly don't disagree with that :-) But we've had some folks at Google who were reluctant to use assertTrue/assertFalse because of this. But sure, I'll send a pull request for this. |
😄 |
You can provide a message to
This message is shown in the stack trace.
|
@stefanbirkner Yes, I'm well aware of that, but why not show a reasonable error message for the default case too? We already do this for assertEquals(expected, actual)...this is so you don't have to write: |
While I agree having a better message for a failure from assertTrue() could be useful, I am not sure if I believe @dsaff had some historical context. |
Just to be clear about the scope of the problem: "Expected but was " is 'much better' because of a stylistic preference for some message rather than none, not because it conveys actual additional information, correct? The original design thought was that making up an error message is a cognitive distraction from the matter at hand. If I say assertTrue(list.isEmpty()), I'm thinking of the state of list, not of a comparison test between booleans. Thus, JUnit has nothing useful to say, and should say nothing. That's the reason behind Chesterton's fence. There's still no really interesting data given in this case mentioned above: assertTrue(user.isHappy()); The reason that assertEquals has a more informative error message is because it has something more informative to say, namely what the actual value and expected values are, which are often not known until runtime. JUnit is a heavily used library, and even minor changes like this, when made to core methods, tend to break someone's test somewhere, where someone is depending on the current behavior of JUnit. I'm fine with doing that if the new behavior is different in a useful way, but I'm not convinced of the usefulness by the argument so far. |
A consequence of the current state is that a number of reasonable and intelligent developers have a personal practice of always writing I also think the answer to David's opening question (last comment) is a clear "yes". The only case in which any actual information is provided is when a test method contains exactly one messageless |
@junit-team/junit-committers What are we going to do about this issue? |
Well, I'm in favor of closing it. Kevin, Kurt, are either of you enthusiastic enough to be counsel for the defense? |
I'm in favor of closing it. |
I do vote up this issue. Other frameworks support showing actual value capability and it's essential. I just couldn't believe JUnit is that inflexible. |
I do vote up this issue since it's important to have a more informative error message for assertTrue and assertFalse. My colleagues and I are reluctant to use assertTrue/assertFalse because of this. |
@ChaminW You can use |
I know its closed but let's vote in emoji |
assertTrue(falseMethod()) gives:
junit.framework.AssertionFailedError
assertEquals(true, falseMethod()) gives:
junit.framework.AssertionFailedError: expected: but was:
Can we add default error messages for assertTrue(boolean)/assertFalse(boolean)?
The easiest thing to do is probably just to call assertEquals(true, condition), etc.
The text was updated successfully, but these errors were encountered: