Nested assumption failures should not cause assertion failures for enclosing assertThrows()

[chrisvest]

Overview

The effect of the org.junit.jupiter.api.Assumptions.assume*() family of methods depends on whether or not the call is inside an assertThrows() lambda or not. I think a failed assumption should always behave the same, i.e. cause the test to be aborted. I think this is a failure of assertThrows() to not propagate the TestAbortedException exception.

Steps to reproduce

The following test fails on the thrown exception being different from the expected, while I think it should instead be ignored:

@Test
void example() {
    assertThrows(IOException.class, () -> assumeTrue(false));
}

Context

• Used versions (Jupiter/Vintage/Platform): JUnit Jupiter version 5.2.0.
• Build Tool/IDE: Maven 3.5.4, Intellij 2018.3.

[sbrannen]

Out of curiosity, what would you expect as the outcome of the following?

@Test
void example() {
    assertThrows(TestAbortedException.class, () -> assumeTrue(false));
}

[sbrannen]

Tentatively slated for 5.4 M1 solely for the purpose of team discussion.

[chrisvest]

That example looks like a trick question, so I wouldn't let any expectation go unverified.

That said, after giving it some though, I would say that the TestAbortedException should still propagate, but I'm not strongly attached to that opinion. Using a try-catch clause remain an alternative for when you really did intend to catch TestAbortedException. On the other hand, why would anyone write such an assertThrows() if they did not mean to verify that the exception was thrown? So it's a question of whether TestAbortedException should be a special case in one, or two ways. It's not clear to me which is more practical.

[sbrannen]

That example looks like a trick question

That's intentional. 😇

[sbrannen]

But it's a valid question: one may wish to implement custom assumption utilities that he or she then wishes to test via assertThrows(...).

[sbrannen]

The next "trick question" is...

What should be done if the Executable passed to assertThrows(...) throws a JUnit 4 AssumptionViolatedException?

Note that we just added support for AssumptionViolatedException in JUnit Jupiter 5.4 M1 (see #1585), but that support resides in junit-jupiter-engine, not in junit-jupiter-api.

[sbrannen]

In other words, it is the job of a TestEngine to decide what kinds of exceptions result in an "aborted test". Conversely, it is (typically) not the job of an assertion framework to make that decision.

[chrisvest]

The assertion framework obviously can't make decisions if it doesn't have the information that is required to do so. Having the test engine communicate this to the assertion framework in some way sounds troublesome. This would also require some common API ground between the two, and I don't know if that exists after opentest4j has been ruled out.

On the other hand, the test engine could inspect the cause of AssertionFailedError for ignore signals. That would work in this particular case, but potentially introduce very surprising behaviours in other cases that happen to stumble into this pattern by accident.

Sounds like this is going to stay "implementation defined"?

[sbrannen]

Sounds like this is going to stay "implementation defined"?

That's correct.

Team Decision: Close this issue as "works as designed".