SonarQube issue "Add at least one assertion to this test case" for unit test with assertions?

Question

I'm having issues with SonarQube raising issues with several of my unit tests, prompting the following issue:

Add at least one assertion to this test case.

Each test case resembles this format (where a number of assertions are delegated to a method with common assertions, to avoid duplication):

@Test
public void companyNameOneTooLong() throws Exception {
    AddressFormBean formBean = getValidBean();
    formBean.setCompanyNameOne("123456789012345678901234567890123456");

    assertViolation(validator.validate(formBean), "companyNameOne", "length must be between 0 and 35");
}

private void assertViolation(Set<ConstraintViolation<AddressFormBean>> violations, String fieldname, String message) {
    assertThat(violations, hasSize(1));
    assertEquals(fieldname, violations.iterator().next().getPropertyPath().iterator().next().getName());
    assertEquals(message, violations.iterator().next().getMessage());
}

Now, obviously I could just pull the three assertions out of the private method and put them in the test method - but I'm performing the same checks (on different fields) multiple times.

So, I thought I'd try to emulate the behaviour of the assertion methods, by (re) throwing an AssertionError:

private void assertViolation(Set<ConstraintViolation<AddressFormBean>> violations, String fieldname, String message) throws AssertionError {
    try {
        assertThat(violations, hasSize(1));
        assertEquals(fieldname, violations.iterator().next().getPropertyPath().iterator().next().getName());
        assertEquals(message, violations.iterator().next().getMessage());
    } catch (AssertionError e) {
        throw e;
    }
 }

Unfortunately, this approach does not work either.

What's special about the JUnit assert methods / what is SonarQube looking for specifically to check that an assertion has been made for each test?

Alternatively - are there other approaches to achieve the same end result (avoiding duplicating the shared assertion code over and over)?

Maybe you could rewrite your method to return a `Matcher`, such that you would have `assertThat(violations, hasViolation(field, message))`? — Didier L, Aug 04 '16 at 10:04
@DidierL Interesting idea - can't see why not. Would you like to post your idea as an answer? — Michael, Aug 04 '16 at 10:27
I am not used to implement matchers but this does not exactly answer your question as to what Sonar is looking for... — Didier L, Aug 04 '16 at 10:30
@DidierL No worries. In any case I've updated my question to allow more flexibility in potential answers. — Michael, Aug 04 '16 at 10:50
Related question: [Is duplicated code more tolerable in unit tests?](https://stackoverflow.com/questions/129693/is-duplicated-code-more-tolerable-in-unit-tests). — Jacob van Lingen, Nov 18 '20 at 07:45

score 18 · Accepted Answer · edited Aug 04 '16 at 12:33

18

The rule S2699 (Tests should include assertions) from the SonarQube Java Analyzer does not perform cross-procedural analysis and only explore body of methods being identified as test method (usually annotated with @Test).

Consequently, if the only assertions which will be called when executing the test method are done by a dedicated method (to avoid duplication), then the rule will raise an issue. This is a known limitation of the rule and we will deal with it only when we will be able to efficiently perform cross-procedural analysis.

Regarding the issues raised by SonarQube on such cases, you can safely mark them as Won't Fix.

Regarding the detected assertions, the rule consider as assertions the usual assert/fail/verify/expect methods from the following (unit test) frameworks :

JUnit
Fest (1.x & 2.x)
AssertJ
Hamcrest
Mockito
Spring
EasyMock

edited Aug 04 '16 at 12:33

janos

120,954
29
226
236

answered Aug 04 '16 at 11:42

Wohops

3,071
18
29

3

Should the message perhaps be reworded to something like "_Add at least one assertion from a supported framework to this test case._" to make it more intuitive? – Michael Aug 04 '16 at 14:35
Using SonarQube 5.6; might I then expect a org.junit.Assert.assertNotNull(x) followed by assertEquals("ack", x.getText) to be OK? SonarQube is complaining that x, in x.getText(), might be null. – Richard Sitze Apr 25 '18 at 18:27
@RichardSitze Your problem is not directly related to this question. Please open a new thread in our ML, and specify the version of your SonarJava plugin: https://groups.google.com/forum/#!forum/sonarqube – Wohops Apr 30 '18 at 07:59
1

@Michael-SonarSourceTeam we are seeing results that appear to show that the Analyzer is now performing cross procedural analysis in the OP's scenario, however cannot find anything with google to confirm this. Are you able to provide an update? – Martin Bamford Jan 07 '20 at 13:22
This doesn't answer the question. How does SonarQube identify that the methods are from those frameworks? Presumably you meant to say that it looks for the fully-qualified class name of the class those assert/fail/verify/expect methods are on to have the well-known package names associated with those frameworks (e.g. org.junit for JUnit), but you didn't actually say that. This is a horrible way to identify assertions, as the question clearly shows. – David Conrad Apr 07 '22 at 19:17

score 11 · Answer 2 · edited Oct 23 '21 at 14:42

11

If you don't expect any exception to be throw from your test, this can be a workaround:

@Test(expected = Test.None.class /* no exception expected */)

Alternatively, you can suppress the warning for the test method/test class:

@SuppressWarnings("squid:S2699")

edited Oct 23 '21 at 14:42

GOTO 0

42,323
22
125
158

answered Oct 14 '20 at 22:35

Chin

19,717
37
107
164

1

I think the annotation is plural "SuppressWarnings" (could not edit it because of the 6 minimum characters rule) – Romano Aug 12 '21 at 18:32

score 2 · Answer 3 · answered Mar 21 '21 at 09:54

Sometimes you don't need to have any code or assertation, for example, the test of load the context of spring boot successfully. In this case, to prevent Sonar issue when you don't expect any exception to be throw from your test, you can use this part of the code:

@Test
void contextLoads() {
    Assertions.assertDoesNotThrow(this::doNotThrowException);
}

private void doNotThrowException(){
    //This method will never throw exception
}

score 2 · Answer 4 · answered Apr 07 '22 at 19:28

One thing I have done in the past is to have the helper method return true, and assert on that:

@Test
public void testSomeThings() {
    Thing expected = // . . .

    Thing actual = service.methodReturningThing(42);

    assertTrue(assertViolation(expected, actual));
}

private boolean assertViolation(Thing expected, Thing actual) {
    assertEquals(expected.getName(), actual.getName());
    assertEquals(expected.getQuest(), actual.getQuest());
    assertEquals(expected.getFavoriteColor(), actual.getFavoriteColor());
    return true;
}

I hate this, but I hate duplicated code even more.

The other thing we've done at times, is to simply mark any such objections from SonarQube as Won't Fix, but I hate that, too.

SonarQube issue "Add at least one assertion to this test case" for unit test with assertions?

4 Answers4