False Positive: "Tests should include assertions" squid:S2699

[Karl Gohlke]

In my opinion I do have an assertion right next to the annotation. Did I miss something?
				@Test(expected = NullPointerException.class)
				public void testTimetokenNull() throws Exception { Add at least one assertion to this test case.  41 minutes ago    L19    Critical     Open    Not assigned    Not planned    10min debt   Comment  junit
				builder.build();
				}
				}

[Francis Galiegue]

The problem as far as I see here is that the .build() method here may fail with an exception (and, by the way, Exception is too broad imho; it may go without saying but RuntimeException extends Exception therefore all RuntimeExceptions will also be thrown); but in this case the test will end up as an _error_, not a failure.

Now, how to not trigger false positives in this case... Well, with TestNG it's relatively simple:

@Test(expectedException = SomeException.class)

public void myTest()

    throws SomeException

{

    // whatever

}

With JUnit, no idea... This SO post gives some ideas:

http://stackoverflow.com/a/16725280

And then you should not really be a slave to your static source code analyzer so what to do in this case would really be to rewrite the test:

@Test

public void testTimeTokenNull()

    throws Exception

{

    try {

        builder.build();

        fail("Hey, I should have had an NPE here");

    } catch (NullPointerException expected) {

        assertTrue(true);

    }

}

[Karl Gohlke]

Sry your recommendation according to Junit ... no way! The idiom @Test(expected=XYException.class) should fix that. Introducing new bolierplate to circumvent that is not a solution. 

Better to swithc off the rule!

[Francis Galiegue]

On Tuesday, July 14, 2015 at 4:30:11 PM UTC+2, Karl Gohlke wrote:

Sry your recommendation according to Junit ... no way! The idiom @Test(expected=XYException.class) should fix that. Introducing new bolierplate to circumvent that is not a solution. 

Think twice about the difference between a test _assertion failure_ and a test _error_.

The reasoning behind the rule here is that tests should have at least one assertion so that on failure it triggers an assertion error, therefore a test failure; if your test fails by throwing an exception then it is a test _error_, which is not quite the same thing. 

[Michel Pawlak]

@Karl,

In order to focus on your original question and code snippet, what you wrote contains an assertion (you expect a NullPointerException). If S2699 raises violations for missing assertions it indeed looks like a false positive. A quick look rule S2699 code, we can see that the @Test(expected=...) syntax is not considered as an assertion, but it should be considered as such.

Regards,

Michel

[Nicolas Peru]

Hi all, 

Thanks for the feedback.

I agree with Michel here, thanks for summing it up. Ticket created : http://jira.sonarsource.com/browse/SONARJAVA-1189 

Cheers,

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/57799116-2839-4b99-8aff-7a7bd346d0a3%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.