Password4j/password4j

Move assertions into separate method or use assertThrows or try-catch instead.

firaja opened this issue · 2 comments

From sonarcloud report:

When testing exception via @Test annotation, having additional assertions inside that test method can be problematic because any code after the raised exception will not be executed. It will prevent you to test the state of the program after the raised exception and, at worst, make you misleadingly think that it is executed.

You should consider moving any assertions into a separate test method where possible, or using org.junit.Assert.assertThrows instead.

Alternatively, you could use try-catch idiom for JUnit version < 4.13 or if your project does not support lambdas.

Compliant

// This test correctly fails.
@Test
public void testToString() {
    Object obj = get();
    Assert.assertThrows(IndexOutOfBoundsException.class, () -> obj.toString());
    Assert.assertEquals(0, 1);
}

Hey @firaja , I will work on this issue

@Test
    public void testPBKDF2WrongCheck2()
    {
        // GIVEN
        String hashed = "$3$42949672960256$YWJj$/WTQfTTc8Hg8GlplP0LthpgdElUG+I3MyuvK8MI4MnQ=";
        String badHash = "$342949672960256$YWJj$/WTQfTTc8Hg8GlplP0LthpgdElUG+I3MyuvK8MI4MnQ=";
        String userSubmittedPassword = "password";

        // WHEN
        HashingFunction strategy = CompressedPBKDF2Function.getInstanceFromHash(hashed);

        // THEN
        BadParametersException ex = assertThrows(BadParametersException.class,()->{
            strategy.check(userSubmittedPassword, badHash);
        });
        assertEquals("`" + hashed + "` is not a valid hash",ex.getMessage());
        Assert.assertTrue(strategy.check(userSubmittedPassword, badHash));
    }

will it be fine to test expected exception like above code

Hello @ganeshannt

sounds good to me, thank you!