Maven doesn't show correct test status when ITestResult status is changed programmatically from TestNG Listeners. Affects all versions since 7.5

[TripleG]

TestNG Version

7.5 and all above (including latest 7.9.0)

Expected behavior

Maven/IntelliJ should show same test result statuses as in testng-results.xml

Actual behavior

When using ITestListener to change the final test status via result.setStatus(), IntelliJ and Maven runners do not show same result as TestNG.
testng-results.xml always shows the correct status that is set through the listener.

Issue is reproducible on the following runners

• Maven
• IntelliJ

Test case sample

As a reference to IntelliJ runner, I am putting link to a bug that I opened couple of months ago in IntelliJ tracker (at the time I thought it is strictly IntelliJ bug) https://youtrack.jetbrains.com/issue/IDEA-339020/TestNG-Runner-doesnt-show-correct-status-of-tests-if-their-status-is-changed-via-ITestListener
But now as I see same behavior for maven, I believe the root case is the same.

Very small code example is attached:
testng_maven_bug_sample_project.zip

Here are also 2 screenshots to show the case:

Version 7.5(same behavior for all up to 7.9.0) - maven shows false positive test results

Version 7.4 - maven shows correct test results

[krmahadevan]

@TripleG - I am not quite sure I u'stand why you would want to change the test result to failure from an onTestSuccess() method.

Now coming to this issue, I am not quite sure as to what is going on here, but my guess is that this is perhaps related to Maven surefire plugin.

Here's what I did with your sample project.

• Bumped up the dependency to 7.9.0
• Created a test main class that looks like below and executed it. You can see the test results as well

package all_tests;

import org.testng.TestNG;

import java.util.Collections;

public class TestMain {

    public static void main(String[] args) {
        TestNG testng = new TestNG();
        testng.setTestSuites(Collections.singletonList("src/test/resources/all_tests_testng.xml"));
        testng.setVerbose(2);
        testng.run();
        System.err.println("Exit status = " + testng.getStatus());
    }
}

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Running test1
Setting test status of test [test1] to FAILURE from MyTestListener
Running test2
Setting test status of test [test2] to FAILURE from MyTestListener
FAILED: all_tests.MyTests.test2
java.lang.AssertionError: foo throwable
	at all_tests.MyTestListener.onTestSuccess(MyTestListener.java:13)
	at org.testng.internal.TestListenerHelper.runTestListeners(TestListenerHelper.java:112)
	at org.testng.internal.invokers.TestInvoker.runTestResultListener(TestInvoker.java:263)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:751)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:228)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:63)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:961)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:201)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.testng.TestRunner.privateRun(TestRunner.java:819)
	at org.testng.TestRunner.run(TestRunner.java:619)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:443)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:437)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:397)
	at org.testng.SuiteRunner.run(SuiteRunner.java:336)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1301)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1228)
	at org.testng.TestNG.runSuites(TestNG.java:1134)
	at org.testng.TestNG.run(TestNG.java:1101)
	at all_tests.TestMain.main(TestMain.java:13)

FAILED: all_tests.MyTests.test1
java.lang.AssertionError: foo throwable
	at all_tests.MyTestListener.onTestSuccess(MyTestListener.java:13)
	at org.testng.internal.TestListenerHelper.runTestListeners(TestListenerHelper.java:112)
	at org.testng.internal.invokers.TestInvoker.runTestResultListener(TestInvoker.java:263)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:751)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:228)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:63)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:961)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:201)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.testng.TestRunner.privateRun(TestRunner.java:819)
	at org.testng.TestRunner.run(TestRunner.java:619)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:443)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:437)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:397)
	at org.testng.SuiteRunner.run(SuiteRunner.java:336)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1301)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1228)
	at org.testng.TestNG.runSuites(TestNG.java:1134)
	at org.testng.TestNG.run(TestNG.java:1101)
	at all_tests.TestMain.main(TestMain.java:13)


===============================================
    all-tests
    Tests run: 2, Failures: 2, Skips: 0
===============================================


===============================================
all
Total tests run: 2, Passes: 0, Failures: 2, Skips: 0
===============================================

Exit status = 1

Process finished with exit code 0

So not sure what can be done from TestNG side.

[TripleG]

@krmahadevan I will give more context on why I am using such listener.
I have custom soft asserts, that I want to be evaluated (checked for fails) after method execution ends. That's why I am doing it in that listener, and I need to change the test status to failed, in case any soft asserts have failed.
Same idea that Selenide uses here https://github.com/selenide/selenide/blob/314fd2a2a79fb7e6546cd94d28c9bf9227885093/modules/testng/src/main/java/com/codeborne/selenide/testng/SoftAsserts.java#L4

The strange thing is that this approach works as expected on 7.4.0 in both IntelliJ and Maven runners, but starting from 7.5, they stop showing the correct statuses.
I checked here, that same issue was mentioned in 2022 https://groups.google.com/g/testng-users/c/ESLiK8xSomc/m/_peFteEeAQAJ

I suspect that something was introduced in 7.5 from TestNG side, because in my sample project same surefire plugin version "breaks" when just changing 7.4 and upper version.

Btw, if I am using the wrong listener for that soft assert idea, I would be grateful if you suggest a better approach to implement it!

[krmahadevan]

I suspect that something was introduced in 7.5 from TestNG side, because in my sample project same surefire plugin version "breaks" when just changing 7.4 and upper version.

If that were the case, then the sample TestMain that I created which uses the TestNG API should also break, but I don't see that happen.

[krmahadevan]

I checked here, that same issue was mentioned in 2022 https://groups.google.com/g/testng-users/c/ESLiK8xSomc/m/_peFteEeAQAJ

Yeah I saw that thread but unfortunately I didn't get the additional context that I was looking for.

I will look at this again tomorrow and see if I can debug more and find out what is going on with Maven executions. In the meantime, if you stumble into any additional information on this, please help share that so that we can get to the bottom of what is going on with this issue.

[krmahadevan]

@TripleG - I spent sometime on this and have narrowed down the root cause to #2558

The larger issue here is that test status alteration is being done via listeners. TestNG has historically never guaranteed the order in which listeners are getting wired into TestNG. So TestNG never guaranteed the order in which the listeners are getting invoked. The only guarantee that TestNG provides is that the listeners will be invoked.

As part of #2558 TestNG provided an order (follow insertion order) for the listeners.

In your sample, you are altering the test status via the onTestSuccess() method and flipping the test to failure.
Maven also has a listener named org.apache.maven.surefire.testng.ConfigurationAwareTestNGReporter which is tracking the test status of each and every method which is later getting used to determine the status of the build.

TestNG 7.5 and upward started honouring the insertion order.
So the maven surefire plugin's listeners were being inserted first (because the programmatic invocation is doing that before it calls testng.run() followed by the listeners provided by your project via the @Listeners annotation.

So even though your listener is flipping the test status properly, maven has already recorded the test status as success.

The fix for this would be to wire in your listener via Service loader integration in TestNG

Doing this would ensure that your listener will first get wired in before the listeners that are getting added explicitly via the maven surefire plugin.

Here's two screenshots that confirms this theory

When adding your sample listener using the @Listeners annotation

Notice in the above screenshot, that org.apache.maven.surefire.testng.ConfigurationAwareTestNGReporter is getting wired in first followed by our sample listener.

Now, when we change the wiring in of the listener by removing the @Listeners annotation and then using the service loader approach, the order looks like below

This will ensure that whatever you altered as the test status will be visible to Maven and decide the build outcome to be failure.

[krmahadevan]

Once we address #2874 then you should be able to seamlessly override these ambiguous behavior of TestNG listener wiring in and execution by a more predictable mechanism of allowing you to define the actual order in which the listeners should be executed.

[TripleG]

@krmahadevan , thank you soooo much! I will try the Service Loader solution!

[TripleG]

@krmahadevan

So TestNG never guaranteed the order in which the listeners are getting invoked.

This means that even using version 7.4, there is still a chance that my listener will be executed after maven's one, correct?

[krmahadevan]

Yes. That is very much possible.

[kool79]

Hi, @TripleG

I recommend you to use another listener to alter test result: IInvokedMethodListener/IInvokedMethodListener2. Please note, this listener is invoked both for @Tests and for @Before... and @After... methods, so you'll be able to define soft-assertions for Before and After as well.

TLDR;
TestNG gives you a big flexibility to change it's behavior by providing ITestResult as a mutable parameter for many of callbacks and configuration methods. But this flexibility requires certain rules to be followed. While technically you can change any data inside ITestResult at any lister/config-method, you must consider some data as 'read-only' after the testng passed some stages.
When TestNG calls 'onTestSuccess/onTestFail/...', it already made a final decision that test is passed/failed/.... The 'onTestSuccess' is a synonym for 'afterTestSuccess'. So you should not change a state of a test (and raleted data) in the TestResult at this stage. Reason is very simple: in another case you'll get unconsistent results from different listeners, attached to the same event, because the order of listeners is unpredictable.