You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/03/01 17:57:39 UTC
[GitHub] [maven-surefire] josephlbarnett opened a new pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
josephlbarnett opened a new pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-789148778
@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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] dankirkd commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
dankirkd commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1010341004
@Tibor17 Now that TestNG 7.5 is out, does this get us any closer to a released fix for this bug?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019384969
@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](https://repository.apache.org/content/repositories/snapshots/org/apache/maven/surefire/surefire/3.0.0-M6-SNAPSHOT/). It is stable and it is related to our master branch, see the [ASF Jenkins CI](https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven-surefire/job/master/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] sullis commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
sullis commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019337970
What is the status on getting this released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585159091
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
Review comment:
>
>
> ok, I can update this to use `Enum.valueOf()`, restrict the allowable values and update the javadoc.
> Is the `Class.forName` ok to get `enumClass`? If not, how should this work given the class doesn't exist in TestNG < 6.9.7?
I think this will work as you have done it. Example, this is similar we already use in another class in the same TestNG provider
```
Class<?> reporterConfig = Class.forName( "org.testng.ReporterConfig" )
```
> I can also use `ReflectionUtils` to call the appropriate `setParallel` method instead of raw reflection, is that ok or is there a better way?
I have already done it for you, see the next comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585143248
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
Review comment:
Pls do not use reflection. We use `org.apache.maven.surefire.api.util.ReflectionUtils` in Surefire.
But in this case I would use the static method `valueOf()` on `java.lang.Enum` and upper case the string this way in a single line:
```
Object parallelEnum = ( ( Enum ) enumClass ).valueOf( parallel.toUpperCase() );
```
https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/xml/XmlSuite.java#L19
If `parallel.toUpperCase()` is other than null, "CLASSES" and "METHODS", then throw `TestSetFailedException`. Can you please extend the Javadoc with one more sentence saying that TestNG supports parallel with two values "classes" and "methods", see https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java#L577.
If you want to add "instances", then we have to open another PR because there are some validation checks and conditions which need to be updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585156013
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
+ Object parallelEnum = resolverMethod.invoke( null, parallel );
+ Method setParallelMethod = XmlSuite.class.getMethod( "setParallel", enumClass );
+ setParallelMethod.invoke( suite, parallelEnum );
Review comment:
This line and the previous line can be substituted by `invokeSetter( Object o, String name, Class<?> value1clazz, Object value )` in the utility class `org.apache.maven.surefire.api.util.ReflectionUtils`.
```
ReflectionUtils.invokeSetter( suite, "setParallel", enumClass, parallelEnum );
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019396603
@josephlbarnett
@dankirkd
@mattsheppard
@sullis
We can make a compromise and cut the version 2.22.3 and cherry pick this patch or maybe some more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-788165049
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?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585143248
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
Review comment:
Pls do not use reflection. We use `org.apache.maven.surefire.api.util.ReflectionUtils` in Surefire.
But in this case I would use the static method `valueOf()` on `java.lang.Enum` and upper case the string this way in a single line:
```
Object parallelEnum = ( ( Enum ) enumClass ).valueOf( parallel.toUpperCase() );
```
https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/xml/XmlSuite.java#L19
If `parallel.toUpperCase()` is other than "CLASSES" and "METHODS", then throw `TestSetFailedException`. Can you please extend the Javadoc with one more sentence saying that TestNG supports parallel with two values "classes" and "methods", see https://github.com/apache/maven-surefire/blob/master/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java#L577.
If you want to add "instances", then we have to open another PR because there are some validation checks and conditions which need to be updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-789813048
Yep that works great, not sure how I missed tryLoadClass when I looked for something like it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] mattsheppard commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
mattsheppard commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1011711965
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-789514427
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 );
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] dankirkd commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
dankirkd commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-966761670
What's the status on getting this released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585149747
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
Review comment:
ok, I can update this to use `Enum.valueOf()`, restrict the allowable values and update the javadoc.
Is the `Class.forName` ok to get `enumClass`? If not, how should this work given the class doesn't exist in TestNG < 6.9.7?
I can also use `ReflectionUtils` to call the appropriate `setParallel` method instead of raw reflection, is that ok or is there a better way?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 merged pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#discussion_r585148158
##########
File path: surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java
##########
@@ -0,0 +1,72 @@
+package org.apache.maven.surefire.testng.conf;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.api.testset.TestSetFailedException;
+import org.testng.xml.XmlSuite;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.Integer.parseInt;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.PARALLEL_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.THREADCOUNT_PROP;
+
+/**
+ * TestNG 7.4.0 configurator. Changed setParallel type to enum value.
+ * Uses reflection since ParallelMode enum doesn't exist in supported
+ * TestNG 5.x versions.
+ *
+ * @since 3.0.0-M6
+ */
+public class TestNG740Configurator extends TestNG60Configurator
+{
+ @Override
+ public void configure( XmlSuite suite, Map<String, String> options )
+ throws TestSetFailedException
+ {
+ String threadCountAsString = options.get( THREADCOUNT_PROP );
+ int threadCount = threadCountAsString == null ? 1 : parseInt( threadCountAsString );
+ suite.setThreadCount( threadCount );
+
+ String parallel = options.get( PARALLEL_PROP );
+ if ( parallel != null )
+ {
+ try
+ {
+ Class enumClass = Class.forName( "org.testng.xml.XmlSuite$ParallelMode" );
+ Method resolverMethod = enumClass.getMethod( "getValidParallel", String.class );
+ Object parallelEnum = resolverMethod.invoke( null, parallel );
+ Method setParallelMethod = XmlSuite.class.getMethod( "setParallel", enumClass );
+ setParallelMethod.invoke( suite, parallelEnum );
+ }
+ catch ( Exception e )
Review comment:
We do not need to have this catch block if we use
```
Object parallelEnum = ( ( Enum ) enumClass ).valueOf( parallel.toUpperCase() );
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019693479
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019384969
@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. 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](https://repository.apache.org/content/repositories/snapshots/org/apache/maven/surefire/surefire/3.0.0-M6-SNAPSHOT/). It is stable and it is related to our master branch, see the [ASF Jenkins CI](https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven-surefire/job/master/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019396603
@josephlbarnett
@dankirkd
@mattsheppard
@sullis
We can make a compromise, cut the version 2.22.3 and cherry pick this patch or maybe some more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019633017
@josephlbarnett
@dankirkd
@mattsheppard
@sullis
Here is the [Git log](https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/release/2.22.3), 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](https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven-surefire/job/release%252F2.22.3/) 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019663004
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](https://issues.apache.org/jira/browse/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] josephlbarnett commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
josephlbarnett commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-790087127
Thx for the quick turaround. Is there an estimated release date for 3.0.0-M6? (just asking, not pressuring)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] dankirkd commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
dankirkd commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019417949
@Tibor17 Where can I find what 2.22.x supports vs. 3.0.0 to better determine whether that would work?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-790027836
@josephlbarnett
Thx for contributing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-790528141
@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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-788453318
>
>
> 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019384969
@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.
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](https://repository.apache.org/content/repositories/snapshots/org/apache/maven/surefire/surefire/3.0.0-M6-SNAPSHOT/). It is stable and it is related to our master branch, see the [ASF Jenkins CI](https://ci-maven.apache.org/job/Maven/job/maven-box/job/maven-surefire/job/master/).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #339: [SUREFIRE-1890] Support TestNG 7.4.0
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #339:
URL: https://github.com/apache/maven-surefire/pull/339#issuecomment-1019633017
@josephlbarnett
@dankirkd
@mattsheppard
@sullis
Here is the [Git log](https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/release/2.22.3), 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org