TestResult.isSuccess() is TRUE when test fails due to expectedExceptions

[ben-manes]

TestNG Version

7.6.1

Expected behavior

IInvokedMethodListener.afterInvocation(method, result) should receive a result with the status FAILURE.

Actual behavior

The status is SUCCESS and getThrowable() is set to a TestException from ExpectedExceptionsHolder.noException.

Is the issue reproducible on runner?

Yes, minimally

• Gradle
• Eclipse

Test case sample

@Test(expectedExceptions = NullPointerException.class)
public void test() {}

Impact

To handle running 10M+ unit tests, my IInvokedMethodListener tries to reclaim memory for successful tests. This avoids out-of-memory errors and long GC pauses, e.g. by replacing test parameters with their stringified version. This also unset the throwable, which causes Gradle's runner to fail with an obscure NPE (gradle/gradle#21336) as it was not defensive to failures without a cause (whereas the Eclipse plugin is tolerant). My fix is to stop nulling out the throwable as it is lightweight enough to retain. However, others might similarly be surprised by the premature success and report it incorrectly in their logic.

Possible Fix

I suspect that adding testResult.setStatus(ITestResult.FAILURE) to the below code would be the correct change. That requires a little rework to retain the originalStatus.

https://github.com/cbeust/testng/blob/b5b3e1d0f8db429b1bb5d1d675ddf5940d19f92c/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java#L865-L872