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

Implement Parallel Method Execution in JUnit-Vintage engine #4242

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 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 @@ -46,6 +46,28 @@ public final class Constants {
@API(status = EXPERIMENTAL, since = "5.12")
public static final String PARALLEL_POOL_SIZE = "junit.vintage.execution.parallel.pool-size";

/**
* Indicates whether parallel execution is enabled for test classes in the JUnit Vintage engine.
*
* <p>Set this property to {@code true} to enable parallel execution of test classes.
* Defaults to {@code false}.
*
* @since 5.13
*/
@API(status = EXPERIMENTAL, since = "5.13")
public static final String PARALLEL_CLASS_EXECUTION = "junit.vintage.execution.parallel.classes";

/**
* Indicates whether parallel execution is enabled for test methods in the JUnit Vintage engine.
*
* <p>Set this property to {@code true} to enable parallel execution of test methods.
* Defaults to {@code false}.
*
* @since 5.13
*/
@API(status = EXPERIMENTAL, since = "5.13")
public static final String PARALLEL_METHOD_EXECUTION = "junit.vintage.execution.parallel.methods";

private Constants() {
/* no-op */
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.TestEngine;
import org.junit.platform.engine.UniqueId;
import org.junit.vintage.engine.descriptor.RunnerScheduler;
import org.junit.vintage.engine.descriptor.RunnerTestDescriptor;
import org.junit.vintage.engine.descriptor.VintageEngineDescriptor;
import org.junit.vintage.engine.discovery.VintageDiscoverer;
Expand All @@ -54,6 +55,9 @@ public final class VintageTestEngine implements TestEngine {
private static final int DEFAULT_THREAD_POOL_SIZE = Runtime.getRuntime().availableProcessors();
private static final int SHUTDOWN_TIMEOUT_SECONDS = 30;

private boolean classes;
private boolean methods;

@Override
public String getId() {
return ENGINE_ID;
Expand Down Expand Up @@ -92,15 +96,23 @@ public void execute(ExecutionRequest request) {

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

if (parallelExecutionEnabled) {
if (executeInParallel(engineDescriptor, engineExecutionListener, request)) {
Thread.currentThread().interrupt();
}
boolean parallelExecutionEnabled = getParallelExecutionEnabled(request);
if (!parallelExecutionEnabled) {
executeSequentially(engineDescriptor, engineExecutionListener);
return;
}
else {

if (!classes && !methods) {
logger.warn(() -> "Parallel execution is enabled but no scope is defined. "
+ "Falling back to sequential execution.");
executeSequentially(engineDescriptor, engineExecutionListener);
return;
}

if (executeInParallel(engineDescriptor, engineExecutionListener, request)) {
Thread.currentThread().interrupt();
}
}

Expand All @@ -109,15 +121,21 @@ private boolean executeInParallel(VintageEngineDescriptor engineDescriptor,
ExecutorService executorService = Executors.newFixedThreadPool(getThreadPoolSize(request));
RunnerExecutor runnerExecutor = new RunnerExecutor(engineExecutionListener);

List<RunnerTestDescriptor> runnerTestDescriptors = collectRunnerTestDescriptors(engineDescriptor,
executorService);

List<CompletableFuture<Void>> futures = new ArrayList<>();
for (Iterator<TestDescriptor> iterator = engineDescriptor.getModifiableChildren().iterator(); iterator.hasNext();) {
TestDescriptor descriptor = iterator.next();
CompletableFuture<Void> future = CompletableFuture.runAsync(() -> {
runnerExecutor.execute((RunnerTestDescriptor) descriptor);
}, executorService);
if (!classes) {
for (RunnerTestDescriptor runnerTestDescriptor : runnerTestDescriptors) {
runnerExecutor.execute(runnerTestDescriptor);
}
return false;
}

for (RunnerTestDescriptor runnerTestDescriptor : runnerTestDescriptors) {
CompletableFuture<Void> future = CompletableFuture.runAsync(
() -> runnerExecutor.execute(runnerTestDescriptor), executorService);
futures.add(future);
iterator.remove();
}

CompletableFuture<Void> allOf = CompletableFuture.allOf(futures.toArray(new CompletableFuture<?>[0]));
Expand All @@ -138,6 +156,41 @@ private boolean executeInParallel(VintageEngineDescriptor engineDescriptor,
return wasInterrupted;
}

private RunnerTestDescriptor parallelMethodExecutor(RunnerTestDescriptor runnerTestDescriptor,
ExecutorService executorService) {
runnerTestDescriptor.setScheduler(new RunnerScheduler() {
@Override
public void schedule(Runnable childStatement) {
executorService.submit(childStatement);
}

@Override
public void finished() {
try {
executorService.shutdown();
executorService.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
Copy link
Member

Choose a reason for hiding this comment

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

I think shutting down the thread pool here is not correct because this will be called for each runner plus we're also using it if classes is true as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with the comment you provided.

Based on that, I removed the shutdown and only used the awaitTermination method.
Then, I added a conditional statement to check if it completes within the given time. e92e834

Upon testing, I found that not all tests finished within the specified time.

As a result, I implemented the finished method to do nothing, but this caused the assertion comparing the start and end times to fail.

Could you provide a hint about what kind of work might need to be done inside the finished method?

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to wait for all futures scheduled for a particular runner. I have addressed that in the commit I've pushed to your branch just now.

}
catch (InterruptedException e) {
logger.warn(e, () -> "Interruption while waiting for parallel test execution to finish");
}
}
});

return runnerTestDescriptor;
}

private List<RunnerTestDescriptor> collectRunnerTestDescriptors(VintageEngineDescriptor engineDescriptor,
ExecutorService executorService) {
List<RunnerTestDescriptor> runnerTestDescriptors = new ArrayList<>();
for (TestDescriptor descriptor : engineDescriptor.getModifiableChildren()) {
RunnerTestDescriptor runnerTestDescriptor = (RunnerTestDescriptor) descriptor;
if (methods) {
runnerTestDescriptors.add(parallelMethodExecutor(runnerTestDescriptor, executorService));
}
}
return runnerTestDescriptors;
}

private void shutdownExecutorService(ExecutorService executorService) {
try {
executorService.shutdown();
Expand Down Expand Up @@ -165,6 +218,11 @@ private boolean getParallelExecutionEnabled(ExecutionRequest request) {
return request.getConfigurationParameters().getBoolean(PARALLEL_EXECUTION_ENABLED).orElse(false);
}

private void initializeParallelExecution(ExecutionRequest request) {
classes = request.getConfigurationParameters().getBoolean(Constants.PARALLEL_CLASS_EXECUTION).orElse(false);
methods = request.getConfigurationParameters().getBoolean(Constants.PARALLEL_METHOD_EXECUTION).orElse(false);
}

private int getThreadPoolSize(ExecutionRequest request) {
Optional<String> poolSize = request.getConfigurationParameters().get(PARALLEL_POOL_SIZE);
if (poolSize.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2015-2025 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.vintage.engine.descriptor;

import static org.apiguardian.api.API.Status.INTERNAL;

import org.apiguardian.api.API;

/**
* Represents a strategy for scheduling when individual test methods
* should be run (in serial or parallel)
*
* @since 5.13
*/
@API(status = INTERNAL, since = "5.13")
public interface RunnerScheduler {
Copy link
Member

Choose a reason for hiding this comment

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

Can't we use org.junit.runners.model.RunnerScheduler directly?

Copy link
Contributor Author

@YongGoose YongGoose Jan 15, 2025

Choose a reason for hiding this comment

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

Directly using org.junit.runners.model.RunnerScheduler seems like a good approach.

However, when running the junit-vintage-engine:testWithoutJUnit4 task, an exception occurs because it fails to find org.junit.runners.model.RunnerScheduler.

  • Exception message
* What went wrong:
Execution failed for task ':junit-vintage-engine:testWithoutJUnit4'.
> Failure during test discovery: Forked test JVM terminated unexpectedly with exit value 1
  Test Distribution has stopped executing any remaining tests and silently skips them!
  For potential reasons why the test JVM exited, check the troubleshooting section at https://gradle.com/help/test-distribution-troubleshooting.
  Standard error from JVM:
      Terminating due to fatal error
      java.lang.NoClassDefFoundError: org/junit/runners/model/RunnerScheduler
      	at java.base/java.lang.Class.getDeclaredConstructors0(Native Method)

So, I’m considering restoring it again. What are your thoughts on that? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I pushed a commit that fixes that. Instead of referencing RunnerScheduler in VintageTestEngine, it passes ExecutorService to RunnerTestDescriptor and creates the RunnerScheduler in there.

/**
* Schedule a child statement to run
*/
void schedule(Runnable childStatement);

/**
* Override to implement any behavior that must occur
* after all children have been scheduled (for example,
* waiting for them all to finish)
*/
void finished();
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ public class RunnerTestDescriptor extends VintageTestDescriptor {
private boolean wasFiltered;
private List<Filter> filters = new ArrayList<>();

private volatile RunnerScheduler scheduler = new RunnerScheduler() {
public void schedule(Runnable childStatement) {
childStatement.run();
}

public void finished() {
// do nothing
}
};

public RunnerTestDescriptor(UniqueId uniqueId, Class<?> testClass, Runner runner, boolean ignored) {
super(uniqueId, runner.getDescription(), testClass.getSimpleName(), ClassSource.from(testClass));
this.runner = runner;
Expand Down Expand Up @@ -161,6 +171,10 @@ public boolean isIgnored() {
return ignored;
}

public void setScheduler(RunnerScheduler scheduler) {
this.scheduler = scheduler;
}
YongGoose marked this conversation as resolved.
Show resolved Hide resolved

private static class ExcludeDescriptionFilter extends Filter {

private final Description description;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
import static org.junit.platform.testkit.engine.EventConditions.event;
import static org.junit.platform.testkit.engine.EventConditions.finishedSuccessfully;
import static org.junit.platform.testkit.engine.EventConditions.started;
import static org.junit.vintage.engine.Constants.PARALLEL_CLASS_EXECUTION;
import static org.junit.vintage.engine.Constants.PARALLEL_EXECUTION_ENABLED;
import static org.junit.vintage.engine.Constants.PARALLEL_METHOD_EXECUTION;
import static org.junit.vintage.engine.Constants.PARALLEL_POOL_SIZE;
import static org.junit.vintage.engine.descriptor.VintageTestDescriptor.SEGMENT_TYPE_RUNNER;
import static org.junit.vintage.engine.samples.junit4.JUnit4ParallelTestCase.AbstractBlockingTestCase;
Expand Down Expand Up @@ -100,6 +102,8 @@ private static LauncherDiscoveryRequest request(int poolSize, Class<?>... testCl
.selectors(classSelectors) //
.configurationParameter(PARALLEL_EXECUTION_ENABLED, String.valueOf(true)) //
.configurationParameter(PARALLEL_POOL_SIZE, String.valueOf(poolSize)) //
.configurationParameter(PARALLEL_CLASS_EXECUTION, String.valueOf(true)) //
.configurationParameter(PARALLEL_METHOD_EXECUTION, String.valueOf(true)) //
.build();
}

Expand Down
Loading