Visitar URL original
Fix out of memory errors with big JUnit 4 suites (#3511) by olamy · Pull Request #3515 · apache/maven-surefire · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@
import static org.apache.maven.plugin.surefire.util.DependencyScanner.filter;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.EXCLUDE_JUNIT5_ENGINES_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.INCLUDE_JUNIT5_ENGINES_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT4_ONLY_DETECTED;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT_VINTAGE_DETECTED;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.RUN_ORDER_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.RUN_ORDER_RANDOM_SEED_PROP;
Expand Down Expand Up @@ -3235,6 +3236,12 @@ public Set<Artifact> getProviderClasspath() throws MojoExecutionException {
&& !testDeps.containsKey("org.junit.vintage:junit-vintage-engine")
&& !pluginDeps.containsKey("org.junit.vintage:junit-vintage-engine")) {
getProperties().setProperty(JUNIT_VINTAGE_DETECTED, "true");
if (testNgArtifact == null
&& !hasDependencyJupiterAPI(testDeps)
&& !hasDependencyPlatformEngine(testDeps)
&& !hasDependencyPlatformEngine(pluginDeps)) {
getProperties().setProperty(JUNIT4_ONLY_DETECTED, "true");
}
if (!hasDependencyPlatformEngine(providerArtifacts)) {
// TODO exclude transitive deps (hamcrest etc...)
consoleLogger.debug("Test dependencies contain JUnit4. Resolving " + engineCoordinates + ":"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ public class ProviderParameterNames {

public static final String JUNIT_VINTAGE_DETECTED = "junit.vintage.engine.detected";

/**
* Set when the project only has JUnit 3 or 4 tests.
*/
public static final String JUNIT4_ONLY_DETECTED = "junit4.only.detected";

public static final String INCLUDES_SCAN_LIST = "junit.includes.scan.list";

public static final String EXCLUDES_SCAN_LIST = "junit.excludes.scan.list";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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;

/**
* JUnit 4 classes run one at a time again, as they did up to 3.5.x (#3511). Each class shows up as its own suite.
*/
public class Surefire3511JUnit4OneClassAtATimeIT extends SurefireJUnit4IntegrationTestCase {
@Test
void runsEachTestClassOnItsOwn() {
unpack("surefire-3511-junit4-one-class-at-a-time")
.executeTest()
.verifyErrorFree(2)
.verifyTextInLog("Suite started: issue3511.FirstTest")
.verifyTextInLog("Suite started: issue3511.SecondTest");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?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-3511-junit4-one-class-at-a-time</artifactId>
<version>1.0</version>
<name>Test for JUnit 4 classes running one at a time</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>
</properties>

<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${surefire.version}</version>
<configuration>
<runOrder>alphabetical</runOrder>
<properties>
<property>
<name>listener</name>
<value>issue3511.SuiteListener</value>
</property>
</properties>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 issue3511;

import org.junit.Test;

public class FirstTest {
@Test
public void test() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 issue3511;

import org.junit.Test;

public class SecondTest {
@Test
public void test() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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 issue3511;

import org.junit.runner.Description;
import org.junit.runner.notification.RunListener;

public class SuiteListener extends RunListener {
@Override
public void testSuiteStarted(Description description) {
System.out.println("Suite started: " + description.getClassName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ public long indexClass(String clazz) {
return indexClassMethod(clazz, null);
}

/**
* Drops the index of a method that will not run again, so it does not stay in memory.
* Indexing the method again gives it a new index.
*/
public void forgetClassMethod(String clazz, String method) {
testIdMapping.remove(new ClassMethod(requireNonNull(clazz), requireNonNull(method)));
}

public Long getLocalIndex() {
return testLocalMapping.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,16 @@ public void testExistingIndexRestoresThreadLocal() {
assertThat(indexer.indexClassMethod(getClass().getName(), "first")).isEqualTo(first);
assertThat(indexer.getLocalIndex()).isEqualTo(first);
}

@Test
public void forgottenMethodGetsANewIndex() {
ClassMethodIndexer indexer = new ClassMethodIndexer();
long kept = indexer.indexClassMethod(getClass().getName(), "kept");
long forgotten = indexer.indexClassMethod(getClass().getName(), "forgotten");

indexer.forgetClassMethod(getClass().getName(), "forgotten");

assertThat(indexer.indexClassMethod(getClass().getName(), "kept")).isEqualTo(kept);
assertThat(indexer.indexClassMethod(getClass().getName(), "forgotten")).isNotEqualTo(forgotten);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.GROUPS_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.INCLUDES_SCAN_LIST;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.INCLUDE_JUNIT5_ENGINES_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT4_ONLY_DETECTED;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.JUNIT_VINTAGE_DETECTED;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.RUN_ORDER_PROP;
import static org.apache.maven.surefire.api.booter.ProviderParameterNames.RUN_ORDER_RANDOM_SEED_PROP;
Expand Down Expand Up @@ -273,7 +274,7 @@ private void execute(LauncherAdapter launcher, TestsToRun testsToRun, RunListene
testExecutionListeners.add(adapter);
testExecutionListeners.addAll(createTestExecutionListeners());

if (testsToRun.allowEagerReading()) {
if (testsToRun.allowEagerReading() && !runsOneClassAtATime()) {
List<DiscoverySelector> selectors = new ArrayList<>();
testsToRun.iterator().forEachRemaining(c -> selectors.add(selectClass(c.getName())));

Expand All @@ -287,6 +288,17 @@ private void execute(LauncherAdapter launcher, TestsToRun testsToRun, RunListene
}
}

/**
* JUnit 3 and 4 classes run one at a time, as they did up to 3.5.x. Running them all together keeps every
* test in memory, which is too much for big suites (#3511). Classes running in parallel still run together.
*/
private boolean runsOneClassAtATime() {
Map<String, String> configuration = getConfigurationParameters();
boolean parallelClasses = Boolean.parseBoolean(configuration.get("junit.vintage.execution.parallel.enabled"))
&& Boolean.parseBoolean(configuration.get("junit.vintage.execution.parallel.classes"));
return Boolean.parseBoolean(parameters.getProviderProperties().get(JUNIT4_ONLY_DETECTED)) && !parallelClasses;
}

/**
* Instantiates the listeners configured through the {@code listener} provider property.
* A class name may refer to a JUnit Platform {@link TestExecutionListener}, a JUnit 4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,9 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult
switch (testExecutionResult.getStatus()) {
case ABORTED:
if (isTest) {
runListener.testAssumptionFailure(
createReportEntry(testIdentifier, testExecutionResult, elapsed));
SimpleReportEntry aborted = createReportEntry(testIdentifier, testExecutionResult, elapsed);
runListener.testAssumptionFailure(aborted);
forgetTestRunId(testIdentifier, aborted);
} else if (isClass) {
reportAbortedClass(testIdentifier, testExecutionResult, elapsed);
} else {
Expand Down Expand Up @@ -203,6 +204,7 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult
if (isTest) {
SimpleReportEntry succeeded = createReportEntry(testIdentifier, null, elapsed);
runListener.testSucceeded(succeeded);
forgetTestRunId(testIdentifier, succeeded);
if (succeeded.getSourceName() != null) {
classesWithSuccessfulTests.add(succeeded.getSourceName());
}
Expand All @@ -213,6 +215,9 @@ public void executionFinished(TestIdentifier testIdentifier, TestExecutionResult
}
}

if (isClass) {
classContainersWithStartedTests.remove(testIdentifier.getUniqueId());
}
runningTestIdentifiersByUniqueId.remove(testIdentifier.getUniqueId());
}

Expand All @@ -237,7 +242,9 @@ private void reportAbortedClass(
runListener.testAssumptionFailure(createReportEntry(testIdentifier, testExecutionResult, elapsed));
} else {
for (TestIdentifier test : targetedTests) {
runListener.testAssumptionFailure(createReportEntry(test, testExecutionResult, null));
SimpleReportEntry aborted = createReportEntry(test, testExecutionResult, null);
runListener.testAssumptionFailure(aborted);
forgetTestRunId(test, aborted);
}
}

Expand All @@ -260,6 +267,16 @@ private void recordStartedTest(TestIdentifier testIdentifier) {
}
}

/**
* A test that passed or was skipped will not run again, so we can forget its id. Keeping all of them made big
* suites run out of memory (#3511). Failed tests keep their id for the rerun.
*/
private void forgetTestRunId(TestIdentifier testIdentifier, SimpleReportEntry report) {
if (testIdentifier.isTest()) {
classMethodIndexer.forgetClassMethod(report.getSourceName(), testIdentifier.getUniqueId());
}
}

private static boolean isClassContainer(TestIdentifier testIdentifier) {
return testIdentifier.isContainer()
&& testIdentifier
Expand Down Expand Up @@ -339,11 +356,15 @@ public void executionSkipped(TestIdentifier testIdentifier, String reason) {
SimpleReportEntry report = createReportEntry(testIdentifier);
runListener.testSetStarting(report);
for (TestIdentifier child : testPlan.getChildren(testIdentifier)) {
runListener.testSkipped(createReportEntry(child, null, emptyMap(), reason, null));
SimpleReportEntry skipped = createReportEntry(child, null, emptyMap(), reason, null);
runListener.testSkipped(skipped);
forgetTestRunId(child, skipped);
}
runListener.testSetCompleted(report);
} else {
runListener.testSkipped(createReportEntry(testIdentifier, null, emptyMap(), reason, null));
SimpleReportEntry skipped = createReportEntry(testIdentifier, null, emptyMap(), reason, null);
runListener.testSkipped(skipped);
forgetTestRunId(testIdentifier, skipped);
}
}

Expand Down
Loading
Loading