[SUREFIRE-1890] Support TestNG 7.4.0

[josephlbarnett]

Use reflection to configure parallel arguments,
as the setParallel(String) method has been removed
and the enum equivalent doesn't exist in older
TestNG versions

[josephlbarnett]

Note I'm not sure the best way to write a test for this given it needs to run on at least TestNG 6.9.7+ (based on the enum's existence, I've only tested against TestNG 7.4.0 where it appears to work correctly).

There's also probably some opportunity for a minor refactor to remove the copy / paste of compatible configuration (setting thread count / dataproviderthreadcount ) by abstracting setParallel(), but not sure how worth it that would be?

[Tibor17]

Note I'm not sure the best way to write a test for this given it needs to run on at least TestNG 6.9.7+ (based on the enum's existence, I've only tested against TestNG 7.4.0 where it appears to work correctly).

You can write the integration test, see the module surefire-its. The unit test is possible but i think you need to extract few lines to a private method and test only that (see Whitebox.invokeMethod()).

There's also probably some opportunity for a minor refactor to remove the copy / paste of compatible configuration (setting thread count / dataproviderthreadcount ) by abstracting setParallel(), but not sure how worth it that would be?

Let's refactor this code in another PR.

[josephlbarnett]

@Tibor17 updated as requested and added a simple integration test. Still need to catch a ClassNotFoundException on the Class.forName(), which is also happening in the code you referenced so hope that's ok.

[Tibor17]

I think we can avoid also ClassNotFoundException using this code. Can you please chceck it out?

import static org.apache.maven.surefire.api.util.ReflectionUtils.tryLoadClass;
import static org.apache.maven.surefire.api.util.ReflectionUtils.invokeGetter;
import static org.apache.maven.surefire.api.util.invokeSetter;

Class<?> enumClass = tryLoadClass( XmlSuite.class.getClassLoader(), "org.testng.xml.XmlSuite$ParallelMode" );
Enum<?> parallelEnum = invokeGetter( enumClass, null, "getValidParallel", parallel );
invokeSetter( suite, "setParallel", enumClass, parallelEnum );

[josephlbarnett]

Yep that works great, not sure how I missed tryLoadClass when I looked for something like it.

[Tibor17]

@josephlbarnett
Thx for contributing

[josephlbarnett]

Thx for the quick turaround. Is there an estimated release date for 3.0.0-M6? (just asking, not pressuring)

[Tibor17]

@josephlbarnett
The main goal of M6 is to rework unstable XML reporter which needs to have an update in the class SimpleReportEntry (currently in progress). This can joined together the feature fix for the JUnit5 dynamic tests and the feature request from Cucumber.

[dankirkd]

What's the status on getting this released?

[dankirkd]

@Tibor17 Now that TestNG 7.5 is out, does this get us any closer to a released fix for this bug?

[mattsheppard]

I'd also like to see a release with this fix. Unfortunately all versions of testng prior to 7.5.0 are subject to a number of CVEs in their dependencies which are hard to fix in our environment because of this issue.

The specific CVEs in the last compatible testng version (7.3.0) are as follows if it's at all helpful:

• https://nvd.nist.gov/vuln/detail/CVE-2021-36374
• https://nvd.nist.gov/vuln/detail/CVE-2021-36373
• https://nvd.nist.gov/vuln/detail/CVE-2020-1945
• https://nvd.nist.gov/vuln/detail/CVE-2020-15250
• https://nvd.nist.gov/vuln/detail/CVE-2017-18640

[sullis]

What is the status on getting this released?

[Tibor17]

@josephlbarnett
@dankirkd
@mattsheppard
@sullis
We have to complete important Jira tickets for this project which makes sense for the M6. I am free until Feb 16, so I have a plenty of spare time to support this project, and yes the pressure exists, we know that we have to be very fast.

At last we have fixed three CVEs and now we support JDK 18 which is very important as well.

Meanwhile please use the SNAPSHOT version of the plugins from the ASF repo. It is stable and it is related to our master branch, see the ASF Jenkins CI.

[Tibor17]

@josephlbarnett
@dankirkd
@mattsheppard
@sullis
We can make a compromise, cut the version 2.22.3 and cherry pick this patch or maybe some more.

[dankirkd]

@Tibor17 Where can I find what 2.22.x supports vs. 3.0.0 to better determine whether that would work?

[Tibor17]

@josephlbarnett
@dankirkd
@mattsheppard
@sullis
Here is the Git log, see the last 22 commits in the branch release/2.22.3
We have backported fixes regarding TestNG and more from master (latest 3.0.0-M6). The release with LATEST version would publish the project page including JIRA report but 2.22.3 is not latest version and so we do not use to publish the documentation. We can think about it. The CI is successful but the release vote takes several days and I would kindly ask everybody to participate in the ASF mailing list and put +1 in the Vote. Always it takes a lot of work to make the development and release.

[Tibor17]

The version 2.22.3 was a subject to start a release vote but the vote was cancelled. One of the reasons was the pending issue SUREFIRE-1556. If we do not want to repeat the same scenario, the issue has to be fixed. This fix will be fixed in 2.22.3 and 3.0.0-M6 as well.

[josephlbarnett]

fwiw, we'd prefer something like a 3.0.0-M5.1 if you're not ready to release a 3.0.0-M6, but dont know the details of how you do releases

[dankirkd]

@Tibor17 Can you confirm that 3.0.0-M6 supports TestNG 7.5?

[slawekjaranowski]

@dankirkd 3.0.0-M6 was released last week - you can check.

[Tibor17]

@dankirkd
I can confirm that the Jira issue SUREFIRE-1890 was fixed. If this was the only compatibility issue, the TestNG should work properly.