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
Should the default stub of JUnit asserts like assertNotNull really be NotNull? #6475
Comments
Thanks for your message. Could you take a look at https://checkerframework.org/manual/#annotate-normal-behavior (this is the first search hit for "junit" in the manual) and see whether that is helpful in explaining the rationale? Your comments are welcome.
You are right about that. From a user's point of view, the program has crashed due to misuse of
JUnit 5 annotations appear in file |
I'm just not sure the authors of JUnit share the same thoughts as JUnit is designed to crash in a specific context and it is their library to annotate ideally. Do you think they would have annotated it like that? I would be curious to ping them what their thoughts are on that. My opinion is it doesn't really crash because it is I understand the rationale checker has and I think that part of doc explains it but each library is going to probably have their own slant on that. My concern is what people will do which I did cause I'm lazy and don't use assertNotNull that often is put Regardless I can accept whatever decision as I can plug the stub file I'm just afraid that it will inhibit uptake of checker and or worse inhibition to write tests. |
Perhaps an example of how this came up might shift the mindset. I have a method called @Nullable Object o = findOrNull(input);
if (o == null) {
fail("o should not be null for this test");
} See I think Checker is caught up on the idea because it says "assert" that it is defensive programming when JUnit is not defensive programming like If I did In fact if I have checker setup you should never need Just because something throws an exception when passed Anyway I'm sorry to get more contentious about it but I have hard time believing that the JUnit assertNotNull is not designed for nullables to be passed. |
I find @agentgt's argument that Personally, I would use
In general, if I can't prove something to myself, then it seems like a mistake to ask the checker to prove it for me: usually, the checker is less expressive than the sort of proof that one can do in one's head (this is usually a good thing!). |
I'm not ignoring this, just busy. Thanks for the thoughtful comments! I will reply shortly. |
I am aware of checker/src/main/java/org/checkerframework/checker/nullness/permit-nullness-assertion-exception.astub
But I have to seriously wonder why the default is not the above particularly since JUnit does not actually throw NPE on
assertNotNull
but rather a junit specific assertion exception?It seems like this would hurt uptake of the library and doesn't really offer any advantage. I also could not find the junit4 astub so is this a blanket of all methods named
assertNotNull
?I have feeling this has probably been discussed before but I just could not find it.
The text was updated successfully, but these errors were encountered: