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.

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

Support some level of parallelization in junit-vintage-engine #4135

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
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 @@ -14,10 +14,20 @@
import static org.junit.platform.engine.TestExecutionResult.successful;
import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.ENGINE_ID;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import org.apiguardian.api.API;
import org.junit.platform.commons.logging.Logger;
import org.junit.platform.commons.logging.LoggerFactory;
import org.junit.platform.commons.util.ExceptionUtils;
import org.junit.platform.engine.EngineDiscoveryRequest;
import org.junit.platform.engine.EngineExecutionListener;
import org.junit.platform.engine.ExecutionRequest;
Expand All @@ -37,6 +47,11 @@
@API(status = INTERNAL, since = "4.12")
public final class VintageTestEngine implements TestEngine {

private static final Logger logger = LoggerFactory.getLogger(VintageTestEngine.class);

private static final int DEFAULT_THREAD_POOL_SIZE = Runtime.getRuntime().availableProcessors();
private static final int SHUTDOWN_TIMEOUT_SECONDS = 30;

@Override
public String getId() {
return ENGINE_ID;
Expand Down Expand Up @@ -75,11 +90,81 @@ public void execute(ExecutionRequest request) {

private void executeAllChildren(VintageEngineDescriptor engineDescriptor,
EngineExecutionListener engineExecutionListener) {
boolean parallelExecutionEnabled = getParallelExecutionEnabled();

if (parallelExecutionEnabled) {
executeInParallel(engineDescriptor, engineExecutionListener);
}
else {
executeSequentially(engineDescriptor, engineExecutionListener);
}
}

private void executeInParallel(VintageEngineDescriptor engineDescriptor,
EngineExecutionListener engineExecutionListener) {
ExecutorService executorService = Executors.newFixedThreadPool(getThreadPoolSize());
RunnerExecutor runnerExecutor = new RunnerExecutor(engineExecutionListener);

List<CompletableFuture<Void>> futures = new ArrayList<>();
for (Iterator<TestDescriptor> iterator = engineDescriptor.getModifiableChildren().iterator(); iterator.hasNext();) {
TestDescriptor descriptor = iterator.next();
CompletableFuture<Void> future = CompletableFuture.runAsync(() -> {
RunnerTestDescriptor testDescriptor = (RunnerTestDescriptor) descriptor;
try {
runnerExecutor.execute(testDescriptor);
}
catch (Exception e) {
engineExecutionListener.executionSkipped(testDescriptor, e.getMessage());
}
}, executorService);

futures.add(future);
iterator.remove();
}

CompletableFuture<Void> allOf = CompletableFuture.allOf(futures.toArray(new CompletableFuture<?>[0]));
try {
allOf.get();
}
catch (InterruptedException e) {
logger.warn(e, () -> "Interruption while waiting for parallel test execution to finish");
Thread.currentThread().interrupt();
marcphilipp marked this conversation as resolved.
Show resolved Hide resolved
}
catch (ExecutionException e) {
throw ExceptionUtils.throwAsUncheckedException(e.getCause());
}
finally {
try {
executorService.shutdown();
if (!executorService.awaitTermination(SHUTDOWN_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
logger.warn(() -> "Executor service did not terminate within the specified timeout");
executorService.shutdownNow();
}
}
catch (InterruptedException e) {
logger.warn(e, () -> "Interruption while waiting for executor service to shut down");
Thread.currentThread().interrupt();
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we extract this into a separate method to improve readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea!
I worked on this in the commit 96b31e0

}
}

private void executeSequentially(VintageEngineDescriptor engineDescriptor,
EngineExecutionListener engineExecutionListener) {
RunnerExecutor runnerExecutor = new RunnerExecutor(engineExecutionListener);
for (Iterator<TestDescriptor> iterator = engineDescriptor.getModifiableChildren().iterator(); iterator.hasNext();) {
runnerExecutor.execute((RunnerTestDescriptor) iterator.next());
iterator.remove();
}
}

private boolean getParallelExecutionEnabled() {
// get parallel execution enabled from configuration
return true;
}

private int getThreadPoolSize() {
// get thread pool size from configuration
return DEFAULT_THREAD_POOL_SIZE;
}

}