[Regression] TestNG extension onStart/onFinish called too many times

[lprimak]

Affected version

3.6.0

Bug description

Starting with 3.6.0, TestNG extension onStart() and onFinish() methods get called too many times.
Previous plugin versions would only call those methods one time.
There is one more caveat to this. If only JUnit Jupiter exist (No TestNG tests), but TestNG is on the Classpath,
TestNG extension should not be invoked at all. This is how previous versions would work.
Here, onStart() gets called when there are no tests, which was breaking one of my builds.

Reproducer: https://github.com/lprimak/maven-test-compiler-reproducer
./mvnw package -Dexception.from.testng=false

Messages message:

[INFO] --- surefire:3.6.0:test (default-test) @ maven-testng-reproducer ---
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO] TestNG is present. Adding per default org.junit.support:testng-engine:1.1.0
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
MyTestNGExtension: onStart called for suite: Command line suite
MyTestNGExtension: onFinish called for suite: Command line suite
MyTestNGExtension: onStart called for suite: Command line suite
MyTestNGExtension: onFinish called for suite: Command line suite
MyTestNGExtension: onStart called for suite: Command line suite
MyTestNGExtension: onFinish called for suite: Command line suite
[INFO] Running com.flowlogix.maven.jupiter.JupiterTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 s -- in com.flowlogix.maven.jupiter.JupiterTest
MyTestNGExtension: onStart called for suite: Command line suite
[INFO] Running com.flowlogix.maven.testng.NextGenTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.003 s -- in com.flowlogix.maven.testng.NextGenTest
MyTestNGExtension: onFinish called for suite: Command line suite
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
[INFO] 

The methods should only be called one time, as with the previous plugin:
./mvnw package -Pprevious-testng

[INFO] --- surefire:3.5.6:test (default-test) @ maven-testng-reproducer ---
[INFO] Using configured provider org.apache.maven.surefire.testng.TestNGProvider
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running TestSuite
MyTestNGExtension: onStart called for suite: Surefire suite
MyTestNGExtension: onFinish called for suite: Surefire suite
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.141 s -- in TestSuite

No TestNG extension's methods should be called, as with the previous plugin, since no TestNG tests get found or executed:
./mvnw package -Pprevious-plugins

[INFO] --- surefire:3.5.6:test (default-test) @ maven-testng-reproducer ---
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.flowlogix.maven.jupiter.JupiterTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 s -- in com.flowlogix.maven.jupiter.JupiterTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

The issue is reproducible via maven 3.9.16 and 4-rc-6

[mpkorstanje]

Since 3.6.0-M1 Surefire uses the junit platform runner to run junit 3, junit 4 and TestNG tests via vintage engine and testng engine (#3179).

The testng engine executes testng with a dry run to discover tests. So your ISuiteListener implementation should be invoked more than once. Up to two times for discovery and once for execution.

Though I wouldn't expect it to be invoked 3 times prior to execution.

@olamy does Surefire do discovery one test class/method at a time? I haven't looked at the code in a while.

[olamy]

@mpkorstanje, yes, that's the problem. I have been working on fix already and will have a PR soon

[olamy]

possible fix here #3470

[lprimak]

I put comments into the PR.