Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SUREFIRE-2252] - Fix JUnit5 reporting when tests are executed in parallel #772

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[SUREFIRE-2252] - Fix JUnit5 reporting when tests are executed in parallel #772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2252JUnit5IT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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.OutputValidator;
import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
import org.junit.Test;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

/**
* Integration tests for SUREFIRE-2252
*/
public class Surefire2252JUnit5IT extends SurefireJUnit4IntegrationTestCase {
@Test
public void testJUnit5() throws Exception {
unpack("surefire-2252-junit5-parallel")
.executeTest()
.verifyTextInLog(
"Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider")
.assertThatLogLine(containsString("Tests run: 1, Failures: 0, Errors: 0, Skipped: 0"), is(2));
}

@Test
public void testJUnit5Xml() {
OutputValidator validator =
unpack("surefire-2252-junit5-parallel-xml").executeTest().verifyErrorFree(2);

validator
.getSurefireReportsFile("TEST-pkg.domain.AxTest.xml", UTF_8)
.assertContainsText("tests=\"1\"")
.assertContainsText("errors=\"0\"")
.assertContainsText("skipped=\"0\"")
.assertContainsText("failures=\"0\"");

validator
.getSurefireReportsFile("TEST-pkg.domain.BxTest.xml", UTF_8)
.assertContainsText("tests=\"1\"")
.assertContainsText("errors=\"0\"")
.assertContainsText("skipped=\"0\"")
.assertContainsText("failures=\"0\"");
}
}
68 changes: 68 additions & 0 deletions surefire-its/src/test/resources/surefire-2252-junit5-parallel-xml/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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.example</groupId>
<artifactId>surefire-2252-junit5-parallel-xml</artifactId>
<version>1.0-SNAPSHOT</version>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>${java.specification.version}</maven.compiler.source>
<maven.compiler.target>${java.specification.version}</maven.compiler.target>
</properties>

<dependencies>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<version>5.10.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire.version}</version>
<configuration>
<properties>
<configurationParameters>
junit.jupiter.execution.parallel.enabled = true
junit.jupiter.execution.parallel.mode.default = same_thread
junit.jupiter.execution.parallel.mode.classes.default = concurrent
</configurationParameters>
</properties>
<threadCount>2</threadCount>
<reportFormat>xml</reportFormat>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>

</project>
9 changes: 9 additions & 0 deletions ...src/test/resources/surefire-2252-junit5-parallel-xml/src/test/java/pkg/domain/AxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pkg.domain;

import org.junit.jupiter.api.Test;

class AxTest {
@Test
void test() {
}
}
9 changes: 9 additions & 0 deletions ...src/test/resources/surefire-2252-junit5-parallel-xml/src/test/java/pkg/domain/BxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pkg.domain;

import org.junit.jupiter.api.Test;

class BxTest {
@Test
void test() {
}
}
68 changes: 68 additions & 0 deletions surefire-its/src/test/resources/surefire-2252-junit5-parallel/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?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.example</groupId>
<artifactId>surefire-2252-junit5-parallel</artifactId>
<version>1.0-SNAPSHOT</version>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>${java.specification.version}</maven.compiler.source>
<maven.compiler.target>${java.specification.version}</maven.compiler.target>
</properties>

<dependencies>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<version>5.10.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire.version}</version>
<configuration>
<properties>
<configurationParameters>
junit.jupiter.execution.parallel.enabled = true
junit.jupiter.execution.parallel.mode.default = same_thread
junit.jupiter.execution.parallel.mode.classes.default = concurrent
</configurationParameters>
</properties>
<threadCount>2</threadCount>
<reportFormat>plain</reportFormat>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>

</project>
9 changes: 9 additions & 0 deletions ...its/src/test/resources/surefire-2252-junit5-parallel/src/test/java/pkg/domain/AxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pkg.domain;

import org.junit.jupiter.api.Test;

class AxTest {
@Test
void test() {
}
}
9 changes: 9 additions & 0 deletions ...its/src/test/resources/surefire-2252-junit5-parallel/src/test/java/pkg/domain/BxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package pkg.domain;

import org.junit.jupiter.api.Test;

class BxTest {
@Test
void test() {
}
}
19 changes: 3 additions & 16 deletions ...platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.apache.maven.surefire.api.util.SurefireReflectionException;
import org.apache.maven.surefire.api.util.TestsToRun;
import org.apache.maven.surefire.shared.utils.StringUtils;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.Filter;
import org.junit.platform.launcher.EngineFilter;
import org.junit.platform.launcher.Launcher;
Expand Down Expand Up @@ -172,25 +171,13 @@ private void invokeAllTests(TestsToRun testsToRun, RunListenerAdapter adapter) {
}

private void execute(TestsToRun testsToRun, RunListenerAdapter adapter) {
if (testsToRun.allowEagerReading()) {
List<DiscoverySelector> selectors = new ArrayList<>();
testsToRun.iterator().forEachRemaining(c -> selectors.add(selectClass(c.getName())));

testsToRun.iterator().forEachRemaining(c -> {
LauncherDiscoveryRequestBuilder builder = request()
.filters(filters)
.configurationParameters(configurationParameters)
.selectors(selectors);

.selectors(selectClass(c.getName()));
launcher.execute(builder.build(), adapter);
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this has been collapsed and both cases are identical. Can you explain why this new code is correct for both? I honest cannot assess whether this is wrong or correct. What should happen in your opinion in the eager case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this a bit, I think I am misunderstanding what the eager reading actually means. Could you please clarify its intention? How does an end user set it and where?

In my case here, what is happening is, in a parallel run, the test runs are combined due to eager reading, the classes get executed together and hence tests get reported as a bundle (I'll look into the reporting aspect, maybe that's the issue and not the execution). But in general, I felt there is no need for eager reading for a parallel run. Maybe we need to separate the 2 conditions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this a bit, I think I am misunderstanding what the eager reading actually means. Could you please clarify its intention? How does an end user set it and where?

Unfortunately, I cannot answer these question because I have no idea.

In my case here, what is happening is, in a parallel run, the test runs are combined due to eager reading, the classes get executed together and hence tests get reported as a bundle (I'll look into the reporting aspect, maybe that's the issue and not the execution). But in general, I felt there is no need for eager reading for a parallel run. Maybe we need to separate the 2 conditions?

Is this similar: https://issues.apache.org/jira/browse/SUREFIRE-2195?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 2195 seems like the same issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't know how to proceed because I do not understand the implications :-(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change I see in logs:

[INFO] Running pkg.domain.BxTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.014 s -- in pkg.domain.BxTest
[INFO] pkg.domain.BxTest.test -- Time elapsed: 0.007 s
[INFO] Running pkg.domain.AxTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 s -- in pkg.domain.AxTest
[INFO] pkg.domain.AxTest.test -- Time elapsed: 0 s

but without:

[INFO] Running pkg.domain.AxTest
[INFO] Running pkg.domain.BxTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 s -- in pkg.domain.AxTest
[INFO] pkg.domain.BxTest.test -- Time elapsed: 0.007 s
[INFO] pkg.domain.AxTest.test -- Time elapsed: 0.007 s
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 s -- in pkg.domain.BxTest

so look like tests are executed sequentially not in parallel

So for me problem is something else ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue seems to be in the test listener which isn't parallel safe and is expecting results to trickle in sequentially, unsure of what needs to be fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I linked some of similar issues ... the problem is probbably in StatelessXmlReporter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slawekjaranowski Do you think you can pick up, I have honestly no clue here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One alternate approach I am investigating is to ignore these xmls and use Allure reports instead. They generate a bunch of json files which seem accurate and have all the relevant information.

testsToRun.iterator().forEachRemaining(c -> {
LauncherDiscoveryRequestBuilder builder = request()
.filters(filters)
.configurationParameters(configurationParameters)
.selectors(selectClass(c.getName()));
launcher.execute(builder.build(), adapter);
});
}
});
}

private void closeLauncher() {
Expand Down
72 changes: 58 additions & 14 deletions ...form/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,22 @@ public void allGivenTestsToRunAreInvoked() throws Exception {
TestsToRun testsToRun = newTestsToRun(TestClass1.class, TestClass2.class);
invokeProvider(provider, testsToRun);

assertThat(executionListener.summaries).hasSize(1);
assertThat(executionListener.summaries).hasSize(2);
TestExecutionSummary summary = executionListener.summaries.get(0);
assertEquals(TestClass1.TESTS_FOUND + TestClass2.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass1.TESTS_STARTED + TestClass2.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass1.TESTS_SKIPPED + TestClass2.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass1.TESTS_SUCCEEDED + TestClass2.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass1.TESTS_ABORTED + TestClass2.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass1.TESTS_FAILED + TestClass2.TESTS_FAILED, summary.getTestsFailedCount());
assertEquals(TestClass1.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass1.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass1.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass1.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass1.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass1.TESTS_FAILED, summary.getTestsFailedCount());

summary = executionListener.summaries.get(1);
assertEquals(TestClass2.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass2.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass2.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass2.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass2.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass2.TESTS_FAILED, summary.getTestsFailedCount());
}

@Test
Expand Down Expand Up @@ -249,6 +257,34 @@ public void singleTestClassIsInvokedLazily() throws Exception {
assertEquals(TestClass1.TESTS_FAILED, summary.getTestsFailedCount());
}

@Test
public void multipleTestClassesAreInvokedLazily() throws Exception {
Launcher launcher = LauncherFactory.create();
JUnitPlatformProvider provider = new JUnitPlatformProvider(providerParametersMock(), launcher);

TestPlanSummaryListener executionListener = new TestPlanSummaryListener();
launcher.registerTestExecutionListeners(executionListener);

invokeProvider(provider, newTestsToRunLazily(TestClass1.class, TestClass2.class));

assertThat(executionListener.summaries).hasSize(2);
TestExecutionSummary summary = executionListener.summaries.get(0);
assertEquals(TestClass1.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass1.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass1.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass1.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass1.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass1.TESTS_FAILED, summary.getTestsFailedCount());

summary = executionListener.summaries.get(1);
assertEquals(TestClass2.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass2.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass2.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass2.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass2.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass2.TESTS_FAILED, summary.getTestsFailedCount());
}

@Test
public void failingTestCaseAfterRerun() throws Exception {
Launcher launcher = LauncherFactory.create();
Expand Down Expand Up @@ -682,14 +718,22 @@ public void allDiscoveredTestsAreInvokedForNullArgument() throws Exception {
assertThat(report.getValue().getSourceName()).isEqualTo(TestClass2.class.getName());
assertThat(report.getValue().getName()).isNull();

assertThat(executionListener.summaries).hasSize(1);
assertThat(executionListener.summaries).hasSize(2);
TestExecutionSummary summary = executionListener.summaries.get(0);
assertEquals(TestClass1.TESTS_FOUND + TestClass2.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass1.TESTS_STARTED + TestClass2.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass1.TESTS_SKIPPED + TestClass2.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass1.TESTS_SUCCEEDED + TestClass2.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass1.TESTS_ABORTED + TestClass2.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass1.TESTS_FAILED + TestClass2.TESTS_FAILED, summary.getTestsFailedCount());
assertEquals(TestClass1.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass1.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass1.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass1.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass1.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass1.TESTS_FAILED, summary.getTestsFailedCount());

summary = executionListener.summaries.get(1);
assertEquals(TestClass2.TESTS_FOUND, summary.getTestsFoundCount());
assertEquals(TestClass2.TESTS_STARTED, summary.getTestsStartedCount());
assertEquals(TestClass2.TESTS_SKIPPED, summary.getTestsSkippedCount());
assertEquals(TestClass2.TESTS_SUCCEEDED, summary.getTestsSucceededCount());
assertEquals(TestClass2.TESTS_ABORTED, summary.getTestsAbortedCount());
assertEquals(TestClass2.TESTS_FAILED, summary.getTestsFailedCount());
}

@Test
Expand Down