Quarkus: QuarkusUnitTest: assertException shadows out setExpectedException

[ge0ffrey]

In the testcase below, this unit test is red (which is correct):

            new QuarkusUnitTest()
            ...
            .setExpectedException(ArithmeticException.class); // RED, which is correct

This unit test is green (which is also correct):

            new QuarkusUnitTest()
            ...
            .assertException(throwable -> {
                assertTrue(throwable instanceof IllegalArgumentException);
                assertTrue(throwable.getMessage().contains("scanAnnotatedClasses"));
            }); // GREEN, which is correct

Obviously, combining both of these (a green and a red test) should give me a red result. But this test is green (which is wrong):

            new QuarkusUnitTest()
            ...
            .setExpectedException(ArithmeticException.class);
            .assertException(throwable -> {
                assertTrue(throwable instanceof IllegalArgumentException);
                assertTrue(throwable.getMessage().contains("scanAnnotatedClasses"));
            }); // GREEN, which is wrong! There is no ArithmeticException!

Full source code:

public class OptaPlannerProcessorIllegalXMLTest {

    @RegisterExtension
    static final QuarkusUnitTest config = new QuarkusUnitTest()
            .setExpectedException(IllegalArgumentException.class)
            .overrideConfigKey("quarkus.optaplanner.solver-config-xml",
                    "io/quarkus/optaplanner/illegalScanAnnotatedSolverConfig.xml")
            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
                    .addAsResource("io/quarkus/optaplanner/illegalScanAnnotatedSolverConfig.xml"))
            .setExpectedException(ArithmeticException.class) // Doesn't happen
            .assertException(throwable -> {
                assertTrue(throwable instanceof IllegalArgumentException);
                assertTrue(throwable.getMessage().contains("scanAnnotatedClasses"));
            });

    @Test
    public void scanAnnotatedClasses() {
        // Should not be called, deployment exception should happen first
        Assertions.fail();
    }
}

Proposal A) Check both (do not shadow)
Proposal B) Fail fast if both are used together.

[geoand]

And now you can contribute a fix to this :)

[ge0ffrey]

You want proposal A) or proposal B) to be implemented?

[geoand]

I think B) is more reasonable. A) could perhaps lead to confusion if things fail? With B) I think things will be very clear since we can provide a proper error message

[ge0ffrey]

And with B) we can always go to A) without breaking backwards compatibility. Not so visa versa. When in doubt, leave it out.
I'll take a look at doing B) later this week.

[geoand]

Thanks @ge0ffrey!

[ge0ffrey]

It won't make this week, but it's on my short term todo list.

[ge0ffrey]

Fix PR is here: https://github.com/quarkusio/quarkus/pull/7119

[gastaldi]

Fixed in https://github.com/quarkusio/quarkus/commit/d7490b15d497560808e00f5287f1b6a0aae8c9b2