@DisableIfTestFails vs @BeforeEach

[perlun]

Hi,

Thanks for an interesting project. It seems like @DisableIfTestFails does not currently honor failures in @BeforeEach methods. In other words, if a @BeforeEach method consistently fails, all test methods in the given class will be executed even when this annotation is being used.

Is this by design, or is it an oversight? The reason I ask is that I'm currently considering adopting this annotation to minimize the log output in case of "catastrophic failure" for some of our tests. In some cases, the tests fail as early as in the @BeforeEach method => will still produce significant amount of logging.

[perlun]

Here's what I did as a workaround for the time being. Perhaps it will help someone else. TestContextInitializationFailedException is the particular exception which can be thrown in @BeforeEach handlers in my case. Upstream docs on LifecycleMethodExecutionExceptionHandler can be found here.

Apply to individual tests by annotating them with @ExtendWith(TestContextInitializationFailedInBeforeEachExtension.class).

public class TestContextInitializationFailedInBeforeEachExtension implements LifecycleMethodExecutionExceptionHandler {

    // Note: static context here means errors will only be reported once, for all classes which
    // derive from this base class. This is almost never an issue but deserves to be pointed out.
    private static final AtomicBoolean FIRST_TEST_METHOD = new AtomicBoolean(true);

    @Override
    public void handleBeforeEachMethodExecutionException(ExtensionContext context, Throwable throwable) throws Throwable {
        if (throwable instanceof TestContextInitializationFailedException) {
            if (FIRST_TEST_METHOD.getAndSet(false)) {
                throw throwable;
            }
            else {
                abort("Error previously reported, skipping test.");
            }
        }
        else {
            // Anything else is unknown to use, so make sure it is propagated to the JUnit platform.
            throw throwable;
        }
    }
}

[beatngu13]

Hi @perlun,

thanks for bringing this to our attention. Given #467 and #505, I think this is more or less by design. But including lifecycle methods such as @BeforeEach sounds reasonable to me.

As you suggested, the extension could additionally (beside TestExecutionExceptionHandler) implement LifecycleMethodExecutionExceptionHandler. We could also incorporate this change in a non-breaking manner by using a configuration parameter, for example:

@DisableIfTestFails(onLifecycleMethod = true) // there's already onAssertion

I leave this open for discussion.

[perlun]

We could also incorporate this change in a non-breaking manner by using a configuration parameter, for example:

I think that approach sounds sane. 👍 Doing it like that means we minimize the impact on existing users, and if/when we think it should become the default, it's perhaps suitable to do in the next/a future major version or something.