You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ol...@apache.org on 2026/10/11 04:59:20 UTC
(maven-surefire) branch master updated: Issue #3462 TestNG extension onStart/onFinish called too many times (#3470)
This is an automated email from the ASF dual-hosted git repository.
olamy pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
The following commit(s) were added to refs/heads/master by this push:
new a35e23b1c Issue #3462 TestNG extension onStart/onFinish called too many times (#3470)
a35e23b1c is described below
commit a35e23b1ccc5aa848491cd75469f9a261e719cdf
Author: Olivier Lamy <ol...@apache.org>
AuthorDate: Sun Oct 11 14:59:13 2026 +1000
Issue #3462 TestNG extension onStart/onFinish called too many times (#3470)
* Issue #3462 TestNG extension onStart/onFinish called too many times
* Issue #3462 Skip the TestNG engine when scanning test classes
---------
Signed-off-by: Olivier Lamy <ol...@apache.org>
---
.mvn/extensions.xml | 5 -
.../jiras/Surefire3462TestNgSuiteListenerIT.java | 69 ++++++++
.../resources/surefire-3462-testng-only/pom.xml | 66 ++++++++
.../src/test/java/issue3462/BaseTest.java | 30 ++++
.../src/test/java/issue3462/TestNGTest.java | 31 ++++
.../test/java/issue3462/TrackingSuiteListener.java | 38 +++++
.../META-INF/services/org.testng.ITestNGListener | 1 +
.../surefire-3462-testng-suite-listener/pom.xml | 72 ++++++++
.../src/test/java/issue3462/JupiterTest.java | 31 ++++
.../src/test/java/issue3462/TestNGTest.java | 31 ++++
.../test/java/issue3462/TrackingSuiteListener.java | 39 +++++
.../META-INF/services/org.testng.ITestNGListener | 1 +
.../junitplatform/JUnitPlatformProvider.java | 53 +++++-
.../junitplatform/TestNgTestClassDetector.java | 161 ++++++++++++++++++
.../junitplatform/TestPlanScannerFilter.java | 139 ++++++++++++++--
.../junitplatform/TestNgTestClassDetectorTest.java | 150 +++++++++++++++++
.../junitplatform/TestPlanScannerFilterTest.java | 182 ++++++++++++++++++++-
17 files changed, 1071 insertions(+), 28 deletions(-)
diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml
index 5b611d154..69dabf616 100644
--- a/.mvn/extensions.xml
+++ b/.mvn/extensions.xml
@@ -17,11 +17,6 @@
~ under the License.
-->
<extensions>
- <!--extension>
- <groupId>eu.maveniverse.maven.mimir</groupId>
- <artifactId>extension3</artifactId>
- <version>0.11.2</version>
- </extension-->
<!--
cannot use the cache because of shaded artifact surefire-shared-utils
the primary artifact is in the cache but the shaded one is not replacing it
diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3462TestNgSuiteListenerIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3462TestNgSuiteListenerIT.java
new file mode 100644
index 000000000..f8c1f867d
--- /dev/null
+++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire3462TestNgSuiteListenerIT.java
@@ -0,0 +1,69 @@
+/*
+ * 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.
+ */
+package org.apache.maven.surefire.its.jiras;
+
+import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * A TestNG {@code ISuiteListener} registered through {@code META-INF/services/org.testng.ITestNGListener}
+ * should not be called more than needed.
+ * <p>
+ * The TestNG engine finds tests with a TestNG dry run, and a dry run calls suite listeners just like a real
+ * run. Surefire used to discover each scanned class on its own, so the listener was called once per test
+ * class, plus once for the discovery before the run, plus once for the run itself. The scan no longer uses
+ * the TestNG engine, so we are down to {@value #EXPECTED_SUITE_EVENTS}:
+ * <ol>
+ * <li>the discovery done right before the run,</li>
+ * <li>the run of {@code TestNGTest}.</li>
+ * </ol>
+ * Getting down to one would need a change in the TestNG engine.
+ *
+ * @see <a href="https://github.com/apache/maven-surefire/issues/3462">GitHub issue #3462</a>
+ */
+public class Surefire3462TestNgSuiteListenerIT extends SurefireJUnit4IntegrationTestCase {
+
+ private static final int EXPECTED_SUITE_EVENTS = 2;
+
+ @Test
+ void suiteListenerIsNotInvokedOncePerScannedClass() throws Exception {
+ unpack("surefire-3462-testng-suite-listener")
+ .executeTest()
+ .verifyErrorFree(2)
+ .assertThatLogLine(containsString("SUREFIRE-3462-SUITE-START"), is(EXPECTED_SUITE_EVENTS))
+ .assertThatLogLine(containsString("SUREFIRE-3462-SUITE-FINISH"), is(EXPECTED_SUITE_EVENTS));
+ }
+
+ /**
+ * In a TestNG only project the TestNG engine is the only engine, so skipping it during the scan must
+ * not make JUnit warn that all engines were excluded.
+ */
+ @Test
+ void testNgOnlyProjectScansWithoutWarning() throws Exception {
+ unpack("surefire-3462-testng-only")
+ .executeTest()
+ .verifyErrorFree(1)
+ .verifyTextNotInLog("All TestEngines were excluded")
+ .assertThatLogLine(containsString("SUREFIRE-3462-SUITE-START"), is(EXPECTED_SUITE_EVENTS))
+ .assertThatLogLine(containsString("SUREFIRE-3462-SUITE-FINISH"), is(EXPECTED_SUITE_EVENTS));
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-only/pom.xml b/surefire-its/src/test/resources/surefire-3462-testng-only/pom.xml
new file mode 100644
index 000000000..9f5d139d5
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-only/pom.xml
@@ -0,0 +1,66 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.surefire</groupId>
+ <artifactId>surefire-3462-testng-only</artifactId>
+ <version>1.0</version>
+ <name>Test for: TestNG ISuiteListener in a TestNG only project</name>
+
+ <properties>
+ <maven.compiler.source>1.8</maven.compiler.source>
+ <maven.compiler.target>1.8</maven.compiler.target>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
+ </properties>
+
+ <dependencies>
+ <!-- No testng-engine here on purpose: Surefire adds it when TestNG is on the test class path,
+ and that is what we test. -->
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
+ <version>7.5</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.0</version>
+ <configuration>
+ <encoding>UTF-8</encoding>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>${surefire.version}</version>
+ </plugin>
+ </plugins>
+ </build>
+
+</project>
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/BaseTest.java b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/BaseTest.java
new file mode 100644
index 000000000..53825477c
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/BaseTest.java
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.testng.annotations.BeforeMethod;
+
+/**
+ * Matches the scanner's name pattern, but TestNG skips abstract classes.
+ */
+public abstract class BaseTest {
+
+ @BeforeMethod
+ public void setUp() {}
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TestNGTest.java b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TestNGTest.java
new file mode 100644
index 000000000..305422a55
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TestNGTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+
+public class TestNGTest extends BaseTest {
+
+ @Test
+ public void sanityCheck() {
+ assertTrue(true);
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TrackingSuiteListener.java b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TrackingSuiteListener.java
new file mode 100644
index 000000000..c6b12faa2
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/java/issue3462/TrackingSuiteListener.java
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.testng.ISuite;
+import org.testng.ISuiteListener;
+
+/**
+ * Registered through {@code META-INF/services/org.testng.ITestNGListener}, like in the issue's reproducer.
+ */
+public class TrackingSuiteListener implements ISuiteListener {
+
+ @Override
+ public void onStart(ISuite suite) {
+ System.out.println("SUREFIRE-3462-SUITE-START");
+ }
+
+ @Override
+ public void onFinish(ISuite suite) {
+ System.out.println("SUREFIRE-3462-SUITE-FINISH");
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/resources/META-INF/services/org.testng.ITestNGListener b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/resources/META-INF/services/org.testng.ITestNGListener
new file mode 100644
index 000000000..560aa645f
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-only/src/test/resources/META-INF/services/org.testng.ITestNGListener
@@ -0,0 +1 @@
+issue3462.TrackingSuiteListener
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/pom.xml b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/pom.xml
new file mode 100644
index 000000000..1c53a9e31
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/pom.xml
@@ -0,0 +1,72 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.surefire</groupId>
+ <artifactId>surefire-3462-testng-suite-listener</artifactId>
+ <version>1.0</version>
+ <name>Test for: TestNG ISuiteListener is not invoked more often than necessary</name>
+
+ <properties>
+ <maven.compiler.source>1.8</maven.compiler.source>
+ <maven.compiler.target>1.8</maven.compiler.target>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <version>5.11.4</version>
+ <scope>test</scope>
+ </dependency>
+ <!-- The testng-engine is deliberately not declared here: Surefire adds it automatically
+ as soon as TestNG is on the test class path, which is the code path under test. -->
+ <dependency>
+ <groupId>org.testng</groupId>
+ <artifactId>testng</artifactId>
+ <version>7.5</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.0</version>
+ <configuration>
+ <encoding>UTF-8</encoding>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-surefire-plugin</artifactId>
+ <version>${surefire.version}</version>
+ </plugin>
+ </plugins>
+ </build>
+
+</project>
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/JupiterTest.java b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/JupiterTest.java
new file mode 100644
index 000000000..560d6073c
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/JupiterTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class JupiterTest {
+
+ @Test
+ void sanityCheck() {
+ assertTrue(true);
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TestNGTest.java b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TestNGTest.java
new file mode 100644
index 000000000..61c9b3368
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TestNGTest.java
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+
+public class TestNGTest {
+
+ @Test
+ public void sanityCheck() {
+ assertTrue(true);
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TrackingSuiteListener.java b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TrackingSuiteListener.java
new file mode 100644
index 000000000..73546c9ce
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/java/issue3462/TrackingSuiteListener.java
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+package issue3462;
+
+import org.testng.ISuite;
+import org.testng.ISuiteListener;
+
+/**
+ * Registered through {@code META-INF/services/org.testng.ITestNGListener}, exactly like in the
+ * reproducer attached to the issue.
+ */
+public class TrackingSuiteListener implements ISuiteListener {
+
+ @Override
+ public void onStart(ISuite suite) {
+ System.out.println("SUREFIRE-3462-SUITE-START");
+ }
+
+ @Override
+ public void onFinish(ISuite suite) {
+ System.out.println("SUREFIRE-3462-SUITE-FINISH");
+ }
+}
diff --git a/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/resources/META-INF/services/org.testng.ITestNGListener b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/resources/META-INF/services/org.testng.ITestNGListener
new file mode 100644
index 000000000..560aa645f
--- /dev/null
+++ b/surefire-its/src/test/resources/surefire-3462-testng-suite-listener/src/test/resources/META-INF/services/org.testng.ITestNGListener
@@ -0,0 +1 @@
+issue3462.TrackingSuiteListener
diff --git a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
index 4460df27b..4b15a9366 100644
--- a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
+++ b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
@@ -24,6 +24,8 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashSet;
@@ -114,6 +116,8 @@ public class JUnitPlatformProvider extends AbstractProvider {
private Map<String, String> configurationParameters = new HashMap<>();
+ private boolean excludeTestNgEngine;
+
private final CommandChainReader commandsReader;
public JUnitPlatformProvider(ProviderParameters parameters) {
@@ -215,9 +219,13 @@ public RunResult invoke(Object forkTestSet) throws TestSetFailedException, Repor
try (LauncherSessionAdapter launcherSession = launcherSessionFactory.openSession(cancellationToken)) {
LauncherAdapter launcher = launcherSession.getLauncher();
if (forkTestSet instanceof TestsToRun) {
+ // The TestNG engine is not skipped here on purpose: reading the classes eagerly would
+ // advance the iteration state which TestsToRun exposes through iterated().
invokeAllTests(launcher, (TestsToRun) forkTestSet, adapter);
} else if (forkTestSet instanceof Class) {
- invokeAllTests(launcher, fromClass((Class<?>) forkTestSet), adapter);
+ Class<?> testClass = (Class<?>) forkTestSet;
+ excludeTestNgEngineIfPossible(Collections.singletonList(testClass.getName()));
+ invokeAllTests(launcher, fromClass(testClass), adapter);
} else if (forkTestSet == null) {
invokeAllTests(launcher, scanClasspath(launcher), adapter);
} else {
@@ -237,12 +245,26 @@ private static void setupJunitLogger() {
}
private TestsToRun scanClasspath(LauncherAdapter launcher) {
- TestPlanScannerFilter filter = new TestPlanScannerFilter(launcher, filters);
ScanResult scanResult = parameters.getScanResult();
+ List<String> candidateClassNames = scannedClassNames(scanResult);
+ excludeTestNgEngineIfPossible(candidateClassNames);
+ Set<String> testNgClassNames =
+ TestNgTestClassDetector.findTestNgTestClasses(parameters.getTestClassLoader(), candidateClassNames);
+ TestPlanScannerFilter filter =
+ new TestPlanScannerFilter(launcher, allFilters(), candidateClassNames, testNgClassNames);
TestsToRun scannedClasses = scanResult.applyFilter(filter, parameters.getTestClassLoader());
return parameters.getRunOrderCalculator().orderTestClasses(scannedClasses);
}
+ private static List<String> scannedClassNames(ScanResult scanResult) {
+ int size = scanResult.size();
+ List<String> classNames = new ArrayList<>(size);
+ for (int i = 0; i < size; i++) {
+ classNames.add(scanResult.getClassName(i));
+ }
+ return classNames;
+ }
+
private void invokeAllTests(LauncherAdapter launcher, TestsToRun testsToRun, RunListenerAdapter adapter)
throws TestSetFailedException {
@@ -408,7 +430,32 @@ LauncherDiscoveryRequest buildLauncherDiscoveryRequestForRerunFailures(RunListen
private LauncherDiscoveryRequestBuilder newRequest() {
- return request().filters(filters).configurationParameters(getConfigurationParameters());
+ return request().filters(allFilters()).configurationParameters(getConfigurationParameters());
+ }
+
+ /**
+ * @return the include/exclude filters, extended by an engine filter which keeps the TestNG engine out of
+ * the request when it was proven that it cannot contribute any test
+ */
+ private Filter<?>[] allFilters() {
+ if (!excludeTestNgEngine) {
+ return filters;
+ }
+ Filter<?>[] allFilters = Arrays.copyOf(filters, filters.length + 1);
+ allFilters[filters.length] = EngineFilter.excludeEngines(TestNgTestClassDetector.TESTNG_ENGINE_ID);
+ return allFilters;
+ }
+
+ /**
+ * Excludes the TestNG engine when none of the given classes could be a TestNG test class. The engine must
+ * never be skipped for a class which was not inspected, so this is only ever called with the complete set
+ * of classes which is about to be handed over to the platform.
+ *
+ * @param classNames the names of every class which is about to be handed over to the platform
+ */
+ private void excludeTestNgEngineIfPossible(Collection<String> classNames) {
+ excludeTestNgEngine =
+ TestNgTestClassDetector.canExcludeTestNgEngine(parameters.getTestClassLoader(), classNames);
}
private Filter<?>[] newFilters() {
diff --git a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetector.java b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetector.java
new file mode 100644
index 000000000..d3e68caba
--- /dev/null
+++ b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetector.java
@@ -0,0 +1,161 @@
+/*
+ * 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.
+ */
+package org.apache.maven.surefire.junitplatform;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.apache.maven.surefire.api.util.ReflectionUtils;
+
+/**
+ * Tells whether a class could be a TestNG test class.
+ * <p>
+ * The TestNG engine starts a real TestNG suite whenever it is asked to discover tests, because it uses
+ * TestNG's dry-run mode to tell test classes from other classes. It does so for every class selector it
+ * receives, even for classes which are obviously not TestNG tests, and a started suite notifies every
+ * registered {@code ISuiteListener}. So Surefire leaves the engine out when none of the classes could be a
+ * TestNG test, and it never asks the engine while scanning: TestNG test classes are spotted here instead.
+ * <p>
+ * The detection mirrors the engine's own {@code IsTestNGTestClass}: a class qualifies when
+ * {@code org.testng.annotations.Test} is present on the class hierarchy or directly on one of its methods.
+ *
+ * @since 3.6.1
+ */
+final class TestNgTestClassDetector {
+
+ static final String TESTNG_ENGINE_ID = "testng";
+
+ private static final String TESTNG_TEST_ANNOTATION = "org.testng.annotations.Test";
+
+ private final Class<? extends Annotation> testAnnotation;
+
+ TestNgTestClassDetector(Class<? extends Annotation> testAnnotation) {
+ this.testAnnotation = testAnnotation;
+ }
+
+ /**
+ * @param testClassLoader the class loader of the tests, may be {@code null}
+ * @return a detector, or {@code null} when TestNG is not on the test class path and the engine can
+ * therefore not contribute anything anyway
+ */
+ static TestNgTestClassDetector tryCreate(ClassLoader testClassLoader) {
+ if (testClassLoader == null) {
+ return null;
+ }
+ Class<?> annotation = ReflectionUtils.tryLoadClass(testClassLoader, TESTNG_TEST_ANNOTATION);
+ if (annotation == null || !annotation.isAnnotation()) {
+ return null;
+ }
+ return new TestNgTestClassDetector(annotation.asSubclass(Annotation.class));
+ }
+
+ /**
+ * @param testClassLoader the class loader of the tests
+ * @param classNames the names of every class which is about to be handed over to the platform
+ * @return {@code true} when the TestNG engine can safely be excluded from the discovery request
+ */
+ static boolean canExcludeTestNgEngine(ClassLoader testClassLoader, Collection<String> classNames) {
+ TestNgTestClassDetector detector = tryCreate(testClassLoader);
+ if (detector == null) {
+ return false;
+ }
+ for (String className : classNames) {
+ Class<?> testClass = ReflectionUtils.tryLoadClass(testClassLoader, className);
+ if (testClass == null || detector.isTestNgTestClass(testClass)) {
+ // Unknown classes are left to the engine, only certainty allows to skip it.
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Finds the scanned classes TestNG would run.
+ *
+ * @param testClassLoader the class loader of the tests
+ * @param classNames the scanned class names
+ * @return the TestNG test classes, empty if TestNG is not on the test class path
+ */
+ static Set<String> findTestNgTestClasses(ClassLoader testClassLoader, Collection<String> classNames) {
+ Set<String> testNgClassNames = new LinkedHashSet<>();
+ TestNgTestClassDetector detector = tryCreate(testClassLoader);
+ if (detector == null) {
+ return testNgClassNames;
+ }
+ for (String className : classNames) {
+ Class<?> testClass = ReflectionUtils.tryLoadClass(testClassLoader, className);
+ if (testClass != null && detector.isRunnableTestNgTestClass(testClass)) {
+ testNgClassNames.add(className);
+ }
+ }
+ return testNgClassNames;
+ }
+
+ /**
+ * Like {@link #isTestNgTestClass(Class)}, but TestNG skips abstract classes and interfaces, so we do too.
+ */
+ boolean isRunnableTestNgTestClass(Class<?> candidate) {
+ return !Modifier.isAbstract(candidate.getModifiers()) && isTestNgTestClass(candidate);
+ }
+
+ boolean isTestNgTestClass(Class<?> candidate) {
+ try {
+ return isAnnotatedInHierarchy(candidate) || hasMethodWithTestAnnotation(candidate);
+ } catch (LinkageError e) {
+ // The class cannot be inspected, so it cannot be ruled out either.
+ return true;
+ }
+ }
+
+ private boolean isAnnotatedInHierarchy(Class<?> candidate) {
+ for (Class<?> clazz = candidate; clazz != null && clazz != Object.class; clazz = clazz.getSuperclass()) {
+ if (clazz.getAnnotation(testAnnotation) != null) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean hasMethodWithTestAnnotation(Class<?> candidate) {
+ for (Class<?> clazz = candidate; clazz != null && clazz != Object.class; clazz = clazz.getSuperclass()) {
+ if (hasDeclaredMethodWithTestAnnotation(clazz)) {
+ return true;
+ }
+ for (Class<?> implemented : clazz.getInterfaces()) {
+ if (hasDeclaredMethodWithTestAnnotation(implemented)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private boolean hasDeclaredMethodWithTestAnnotation(Class<?> clazz) {
+ for (Method method : clazz.getDeclaredMethods()) {
+ if (method.getAnnotation(testAnnotation) != null) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilter.java b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilter.java
index f68b4df9d..9a6bd5a2c 100644
--- a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilter.java
+++ b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilter.java
@@ -18,38 +18,155 @@
*/
package org.apache.maven.surefire.junitplatform;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.ServiceConfigurationError;
+import java.util.ServiceLoader;
+import java.util.Set;
+
import org.apache.maven.surefire.api.util.ScannerFilter;
+import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.Filter;
+import org.junit.platform.engine.TestEngine;
+import org.junit.platform.engine.TestSource;
+import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.engine.support.descriptor.MethodSource;
+import org.junit.platform.launcher.EngineFilter;
import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestIdentifier;
import org.junit.platform.launcher.TestPlan;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
/**
+ * Accepts the scanned classes the JUnit Platform can actually run tests for.
+ * <p>
+ * All candidate classes are discovered in a <em>single</em> discovery request instead of one request per
+ * class. Discovery is not free: engines may execute arbitrary user code while discovering, and the TestNG
+ * engine even starts a real TestNG suite because it relies on TestNG's dry-run mode. Discovering once per
+ * scanned class multiplied those side effects by the number of test classes.
+ * <p>
+ * TestNG test classes skip discovery: {@link TestNgTestClassDetector} spots them and they are accepted as
+ * is. The other classes are discovered without the TestNG engine, so the only TestNG dry run left is the
+ * one done right before the tests run.
+ *
* @since 2.22.0
*/
final class TestPlanScannerFilter implements ScannerFilter {
- private final LauncherAdapter launcher;
-
- private final Filter<?>[] includeAndExcludeFilters;
+ private final Set<String> classNamesWithTests;
- TestPlanScannerFilter(LauncherAdapter launcher, Filter<?>[] includeAndExcludeFilters) {
- this.launcher = launcher;
- this.includeAndExcludeFilters = includeAndExcludeFilters;
+ /**
+ * @param launcher discovers the classes which are not TestNG test classes
+ * @param includeAndExcludeFilters the filters for that discovery
+ * @param candidateClassNames the scanned class names
+ * @param testNgClassNames the TestNG test classes, accepted without discovery
+ */
+ TestPlanScannerFilter(
+ LauncherAdapter launcher,
+ Filter<?>[] includeAndExcludeFilters,
+ Collection<String> candidateClassNames,
+ Collection<String> testNgClassNames) {
+ Set<String> otherClassNames = new LinkedHashSet<>(candidateClassNames);
+ otherClassNames.removeAll(testNgClassNames);
+ Filter<?>[] filters = includeAndExcludeFilters;
+ if (!testNgClassNames.isEmpty()) {
+ if (!otherClassNames.isEmpty() && !hasEngineOtherThanTestNg()) {
+ // Nothing but TestNG could run these, and they are not TestNG tests.
+ otherClassNames.clear();
+ }
+ filters = Arrays.copyOf(includeAndExcludeFilters, includeAndExcludeFilters.length + 1);
+ filters[includeAndExcludeFilters.length] =
+ EngineFilter.excludeEngines(TestNgTestClassDetector.TESTNG_ENGINE_ID);
+ }
+ classNamesWithTests = discoverClassNamesWithTests(launcher, filters, otherClassNames);
+ classNamesWithTests.addAll(testNgClassNames);
}
@Override
@SuppressWarnings("rawtypes")
public boolean accept(Class testClass) {
- LauncherDiscoveryRequest discoveryRequest = request()
- .selectors(selectClass(testClass.getName()))
- .filters(includeAndExcludeFilters)
- .build();
+ return classNamesWithTests.contains(testClass.getName());
+ }
+
+ private static Set<String> discoverClassNamesWithTests(
+ LauncherAdapter launcher, Filter<?>[] includeAndExcludeFilters, Collection<String> candidateClassNames) {
+
+ Set<String> candidates = new LinkedHashSet<>(candidateClassNames);
+ if (candidates.isEmpty()) {
+ return new HashSet<>();
+ }
+
+ List<DiscoverySelector> selectors = new ArrayList<>(candidates.size());
+ for (String className : candidates) {
+ selectors.add(selectClass(className));
+ }
+
+ LauncherDiscoveryRequest discoveryRequest =
+ request().selectors(selectors).filters(includeAndExcludeFilters).build();
TestPlan testPlan = launcher.discover(discoveryRequest);
- return testPlan.containsTests();
+ // The launcher prunes every descriptor which neither contains nor may register tests, so a class
+ // which is still part of the test plan is a class the platform has tests for.
+ Set<String> discovered = new HashSet<>();
+ for (TestIdentifier root : testPlan.getRoots()) {
+ collectClassNames(testPlan, root, candidates, discovered);
+ }
+ return discovered;
+ }
+
+ /**
+ * Finds the engines the same way the launcher does. If TestNG is the only one, leaving it out would make
+ * the launcher warn that all engines were excluded.
+ */
+ private static boolean hasEngineOtherThanTestNg() {
+ ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
+ if (classLoader == null) {
+ classLoader = ClassLoader.getSystemClassLoader();
+ }
+ try {
+ for (TestEngine engine : ServiceLoader.load(TestEngine.class, classLoader)) {
+ if (!TestNgTestClassDetector.TESTNG_ENGINE_ID.equals(engine.getId())) {
+ return true;
+ }
+ }
+ return false;
+ } catch (ServiceConfigurationError | LinkageError e) {
+ // Leave broken engines to the launcher, like before.
+ return true;
+ }
+ }
+
+ private static void collectClassNames(
+ TestPlan testPlan, TestIdentifier identifier, Set<String> candidates, Set<String> collected) {
+
+ Optional<String> className = className(identifier);
+ if (className.isPresent() && candidates.contains(className.get())) {
+ collected.add(className.get());
+ }
+ for (TestIdentifier child : testPlan.getChildren(identifier)) {
+ collectClassNames(testPlan, child, candidates, collected);
+ }
+ }
+
+ private static Optional<String> className(TestIdentifier identifier) {
+ return identifier.getSource().map(TestPlanScannerFilter::className);
+ }
+
+ private static String className(TestSource source) {
+ if (source instanceof ClassSource) {
+ return ((ClassSource) source).getClassName();
+ }
+ if (source instanceof MethodSource) {
+ return ((MethodSource) source).getClassName();
+ }
+ return null;
}
}
diff --git a/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetectorTest.java b/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetectorTest.java
new file mode 100644
index 000000000..c234d6487
--- /dev/null
+++ b/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestNgTestClassDetectorTest.java
@@ -0,0 +1,150 @@
+/*
+ * 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.
+ */
+package org.apache.maven.surefire.junitplatform;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+import org.junit.jupiter.api.Test;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Unit tests for {@link TestNgTestClassDetector}.
+ * <p>
+ * TestNG itself is not a dependency of this module, so the detection logic is exercised with a stand-in
+ * annotation. Only the resolution of {@code org.testng.annotations.Test} depends on the real TestNG.
+ */
+class TestNgTestClassDetectorTest {
+
+ private final TestNgTestClassDetector detector = new TestNgTestClassDetector(TestNgLikeTest.class);
+
+ @Test
+ void classWithoutAnnotationIsNotATestNgTestClass() {
+ assertFalse(detector.isTestNgTestClass(PlainClass.class));
+ }
+
+ @Test
+ void classAnnotatedAtTypeLevelIsATestNgTestClass() {
+ assertTrue(detector.isTestNgTestClass(AnnotatedClass.class));
+ }
+
+ @Test
+ void classWithAnnotatedMethodIsATestNgTestClass() {
+ assertTrue(detector.isTestNgTestClass(ClassWithAnnotatedMethod.class));
+ }
+
+ @Test
+ void classInheritingTypeLevelAnnotationIsATestNgTestClass() {
+ assertTrue(detector.isTestNgTestClass(SubclassOfAnnotatedClass.class));
+ }
+
+ @Test
+ void classInheritingAnnotatedMethodIsATestNgTestClass() {
+ assertTrue(detector.isTestNgTestClass(SubclassOfClassWithAnnotatedMethod.class));
+ }
+
+ @Test
+ void classImplementingInterfaceWithAnnotatedMethodIsATestNgTestClass() {
+ assertTrue(detector.isTestNgTestClass(ImplementsAnnotatedInterface.class));
+ }
+
+ @Test
+ void concreteTestNgTestClassIsRunnable() {
+ assertTrue(detector.isRunnableTestNgTestClass(ClassWithAnnotatedMethod.class));
+ }
+
+ @Test
+ void abstractTestNgTestClassIsNotRunnable() {
+ assertFalse(detector.isRunnableTestNgTestClass(AbstractClassWithAnnotatedMethod.class));
+ }
+
+ @Test
+ void interfaceWithAnnotatedMethodIsNotRunnable() {
+ assertFalse(detector.isRunnableTestNgTestClass(AnnotatedInterface.class));
+ }
+
+ @Test
+ void noTestNgTestClassIsFoundWhenTestNgIsNotOnTheClassPath() {
+ assertTrue(TestNgTestClassDetector.findTestNgTestClasses(
+ getClass().getClassLoader(), singletonList(ClassWithAnnotatedMethod.class.getName()))
+ .isEmpty());
+ }
+
+ @Test
+ void engineIsNotExcludedWhenTestNgIsNotOnTheClassPath() {
+ assertFalse(TestNgTestClassDetector.canExcludeTestNgEngine(
+ getClass().getClassLoader(), singletonList(PlainClass.class.getName())));
+ }
+
+ @Test
+ void engineIsNotExcludedWithoutClassLoader() {
+ assertFalse(TestNgTestClassDetector.canExcludeTestNgEngine(null, emptyList()));
+ }
+
+ @Test
+ void detectorIsNotCreatedWhenTestNgIsNotOnTheClassPath() {
+ assertFalse(TestNgTestClassDetector.tryCreate(getClass().getClassLoader()) != null);
+ }
+
+ @Test
+ void engineIsNotExcludedWhenAClassCannotBeLoaded() {
+ assertFalse(TestNgTestClassDetector.canExcludeTestNgEngine(
+ getClass().getClassLoader(), asList(PlainClass.class.getName(), "does.not.Exist")));
+ }
+
+ @Retention(RetentionPolicy.RUNTIME)
+ @interface TestNgLikeTest {}
+
+ static class PlainClass {
+
+ void method() {}
+ }
+
+ @TestNgLikeTest
+ static class AnnotatedClass {}
+
+ static class SubclassOfAnnotatedClass extends AnnotatedClass {}
+
+ static class ClassWithAnnotatedMethod {
+
+ @TestNgLikeTest
+ void test() {}
+ }
+
+ static class SubclassOfClassWithAnnotatedMethod extends ClassWithAnnotatedMethod {}
+
+ abstract static class AbstractClassWithAnnotatedMethod {
+
+ @TestNgLikeTest
+ void test() {}
+ }
+
+ interface AnnotatedInterface {
+
+ @TestNgLikeTest
+ void test();
+ }
+
+ abstract static class ImplementsAnnotatedInterface implements AnnotatedInterface {}
+}
diff --git a/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilterTest.java b/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilterTest.java
index e70cf4a35..3e6c1d3b8 100644
--- a/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilterTest.java
+++ b/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/TestPlanScannerFilterTest.java
@@ -18,6 +18,8 @@
*/
package org.apache.maven.surefire.junitplatform;
+import java.net.URL;
+import java.net.URLClassLoader;
import java.util.List;
import java.util.stream.Stream;
@@ -26,9 +28,19 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.TestFactory;
import org.junit.platform.engine.Filter;
+import org.junit.platform.launcher.EngineFilter;
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryListener;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
import org.junit.platform.launcher.core.LauncherFactory;
+import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static java.util.stream.Collectors.toList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -41,45 +53,197 @@ public class TestPlanScannerFilterTest {
@org.junit.jupiter.api.Test
public void emptyClassIsNotAccepted() {
- assertFalse(newFilter().accept(EmptyClass.class), "does not accept empty class");
+ assertFalse(newFilter(EmptyClass.class).accept(EmptyClass.class), "does not accept empty class");
}
@org.junit.jupiter.api.Test
public void classWithNoTestMethodsIsNotAccepted() {
- assertFalse(newFilter().accept(ClassWithMethods.class), "does not accept class with no @Test methods");
+ assertFalse(
+ newFilter(ClassWithMethods.class).accept(ClassWithMethods.class),
+ "does not accept class with no @Test methods");
}
@org.junit.jupiter.api.Test
public void classWithTestMethodsIsAccepted() {
- assertTrue(newFilter().accept(ClassWithTestMethods.class));
+ assertTrue(newFilter(ClassWithTestMethods.class).accept(ClassWithTestMethods.class));
}
@org.junit.jupiter.api.Test
public void classWithNestedTestClassIsAccepted() {
- assertTrue(newFilter().accept(ClassWithNestedTestClass.class));
+ assertTrue(newFilter(ClassWithNestedTestClass.class).accept(ClassWithNestedTestClass.class));
}
@org.junit.jupiter.api.Test
public void classWithDeeplyNestedTestClassIsAccepted() {
- assertTrue(newFilter().accept(ClassWithDeeplyNestedTestClass.class));
+ assertTrue(newFilter(ClassWithDeeplyNestedTestClass.class).accept(ClassWithDeeplyNestedTestClass.class));
}
@org.junit.jupiter.api.Test
public void classWithTestFactoryIsAccepted() {
- assertTrue(newFilter().accept(ClassWithTestFactory.class));
+ assertTrue(newFilter(ClassWithTestFactory.class).accept(ClassWithTestFactory.class));
}
@org.junit.jupiter.api.Test
public void classWithNestedTestFactoryIsAccepted() {
- assertTrue(newFilter().accept(ClassWithNestedTestFactory.class));
+ assertTrue(newFilter(ClassWithNestedTestFactory.class).accept(ClassWithNestedTestFactory.class));
}
- private static TestPlanScannerFilter newFilter() {
- return new TestPlanScannerFilter(new LauncherAdapter(LauncherFactory.create(), null), new Filter<?>[0]);
+ @org.junit.jupiter.api.Test
+ public void allCandidatesAreDiscoveredInASingleRequest() {
+ CountingLauncher launcher = new CountingLauncher(LauncherFactory.create());
+ TestPlanScannerFilter filter = new TestPlanScannerFilter(
+ new LauncherAdapter(launcher, null),
+ new Filter<?>[0],
+ Stream.of(
+ EmptyClass.class,
+ ClassWithMethods.class,
+ ClassWithTestMethods.class,
+ ClassWithNestedTestClass.class,
+ ClassWithTestFactory.class)
+ .map(Class::getName)
+ .collect(toList()),
+ emptyList());
+
+ assertEquals(1, launcher.discoveries, "discovers all candidate classes at once");
+ assertFalse(filter.accept(EmptyClass.class));
+ assertFalse(filter.accept(ClassWithMethods.class));
+ assertTrue(filter.accept(ClassWithTestMethods.class));
+ assertTrue(filter.accept(ClassWithNestedTestClass.class));
+ assertTrue(filter.accept(ClassWithTestFactory.class));
+ }
+
+ @org.junit.jupiter.api.Test
+ public void nothingIsDiscoveredWithoutCandidates() {
+ CountingLauncher launcher = new CountingLauncher(LauncherFactory.create());
+ TestPlanScannerFilter filter = new TestPlanScannerFilter(
+ new LauncherAdapter(launcher, null), new Filter<?>[0], emptyList(), emptyList());
+
+ assertEquals(0, launcher.discoveries, "does not discover anything without candidates");
+ assertFalse(filter.accept(ClassWithTestMethods.class));
+ }
+
+ @org.junit.jupiter.api.Test
+ public void classIsNotAcceptedWhenOnlyAnotherCandidateHasTests() {
+ assertFalse(newFilter(EmptyClass.class, ClassWithTestMethods.class).accept(EmptyClass.class));
+ }
+
+ @org.junit.jupiter.api.Test
+ public void testNgClassesAreAcceptedWithoutAskingTheTestNgEngine() {
+ CountingLauncher launcher = new CountingLauncher(LauncherFactory.create());
+ TestPlanScannerFilter filter = new TestPlanScannerFilter(
+ new LauncherAdapter(launcher, null),
+ new Filter<?>[0],
+ asList(EmptyClass.class.getName(), ClassWithTestMethods.class.getName(), TestNgClass.class.getName()),
+ singletonList(TestNgClass.class.getName()));
+
+ assertEquals(1, launcher.discoveries, "still discovers the other classes");
+ assertTrue(excludesTestNgEngine(launcher.lastRequest), "the TestNG engine is left out of the discovery");
+ assertTrue(filter.accept(TestNgClass.class));
+ assertTrue(filter.accept(ClassWithTestMethods.class));
+ assertFalse(filter.accept(EmptyClass.class));
+ }
+
+ @org.junit.jupiter.api.Test
+ public void testNgEngineIsNotExcludedWithoutTestNgClasses() {
+ CountingLauncher launcher = new CountingLauncher(LauncherFactory.create());
+ new TestPlanScannerFilter(
+ new LauncherAdapter(launcher, null),
+ new Filter<?>[0],
+ singletonList(ClassWithTestMethods.class.getName()),
+ emptyList());
+
+ assertFalse(excludesTestNgEngine(launcher.lastRequest));
+ }
+
+ @org.junit.jupiter.api.Test
+ public void nothingIsDiscoveredWhenTestNgIsTheOnlyEngine() throws Exception {
+ CountingLauncher launcher = new CountingLauncher(LauncherFactory.create());
+ Thread thread = Thread.currentThread();
+ ClassLoader previous = thread.getContextClassLoader();
+ // This class loader sees no engine at all, which looks the same as TestNG being the only one.
+ try (URLClassLoader noEngines = new URLClassLoader(new URL[0], null)) {
+ thread.setContextClassLoader(noEngines);
+ TestPlanScannerFilter filter = new TestPlanScannerFilter(
+ new LauncherAdapter(launcher, null),
+ new Filter<?>[0],
+ asList(EmptyClass.class.getName(), TestNgClass.class.getName()),
+ singletonList(TestNgClass.class.getName()));
+
+ assertEquals(0, launcher.discoveries, "no engine is left to ask");
+ assertTrue(filter.accept(TestNgClass.class));
+ assertFalse(filter.accept(EmptyClass.class));
+ } finally {
+ thread.setContextClassLoader(previous);
+ }
+ }
+
+ private static boolean excludesTestNgEngine(LauncherDiscoveryRequest request) {
+ for (EngineFilter engineFilter : request.getEngineFilters()) {
+ if (!engineFilter.isIncludeFilter()
+ && engineFilter.getEngineIds().contains(TestNgTestClassDetector.TESTNG_ENGINE_ID)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private static TestPlanScannerFilter newFilter(Class<?>... candidates) {
+ List<String> candidateClassNames =
+ Stream.of(candidates).map(Class::getName).collect(toList());
+ return new TestPlanScannerFilter(
+ new LauncherAdapter(LauncherFactory.create(), null),
+ new Filter<?>[0],
+ candidateClassNames,
+ emptyList());
+ }
+
+ private static final class CountingLauncher implements Launcher {
+
+ private final Launcher delegate;
+
+ private int discoveries;
+
+ private LauncherDiscoveryRequest lastRequest;
+
+ private CountingLauncher(Launcher delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void registerLauncherDiscoveryListeners(LauncherDiscoveryListener... listeners) {
+ delegate.registerLauncherDiscoveryListeners(listeners);
+ }
+
+ @Override
+ public void registerTestExecutionListeners(TestExecutionListener... listeners) {
+ delegate.registerTestExecutionListeners(listeners);
+ }
+
+ @Override
+ public TestPlan discover(LauncherDiscoveryRequest launcherDiscoveryRequest) {
+ discoveries++;
+ lastRequest = launcherDiscoveryRequest;
+ return delegate.discover(launcherDiscoveryRequest);
+ }
+
+ @Override
+ public void execute(LauncherDiscoveryRequest launcherDiscoveryRequest, TestExecutionListener... listeners) {
+ delegate.execute(launcherDiscoveryRequest, listeners);
+ }
+
+ @Override
+ public void execute(TestPlan testPlan, TestExecutionListener... listeners) {
+ delegate.execute(testPlan, listeners);
+ }
}
static class EmptyClass {}
+ /**
+ * Plays a TestNG test class. Being in the TestNG list is enough, no annotation needed.
+ */
+ static class TestNgClass {}
+
static class ClassWithMethods {
void method1() {}