junit-team / junit5

✅ The 5th major version of the programmer-friendly testing framework for Java and the JVM
https://junit.org
Eclipse Public License 2.0
6.42k stars 1.49k forks source link

JUnit parallel executor running too many tests with a fixed policy. #3108

Open OPeyrusse opened 1 year ago

OPeyrusse commented 1 year ago

JUnit only parallel executor service relies on a ForkJoinPool. Unfortunately, this does not play well with code also using ForkJoinPools.

The observed issue is that, when activating parallel tests, JUnit uses ForkJoinPoolHierarchicalTestExecutorService. However, our tests are also using ForkJoinPools and ForkJoinTasks. The orchestration of the test awaits for the completion of those tasks before moving on in the tests. But the issue is that ForkJoinTask and ForkJoinWorkerThread are capable of detecting the use of the FJP framework (here for example) and react to it. As JUnit tasks and the project tasks are in different ForkJoinPools, they cannot help each other. This only results in more tests being started by already running and incomplete tests.

This can be an issue when tests are resource sensitive. For example, it may not be possible to open too many connections to a Database. Tough not explicitly illustrated in this project, we also faced StackOverflowError because of recursive test executions in a single worker. In the following logs, produced by the reproducing project, we can see that two workers are recursively starting tests before finishing any:

ForkJoinPool-1-worker-1 starting a new test. Running: 2
ForkJoinPool-1-worker-2 starting a new test. Running: 1
ForkJoinPool-1-worker-1 starting a new test. Running: 3
ForkJoinPool-1-worker-1 starting a new test. Running: 4
ForkJoinPool-1-worker-1 starting a new test. Running: 5
ForkJoinPool-1-worker-2 starting a new test. Running: 6
ForkJoinPool-1-worker-1 starting a new test. Running: 7
ForkJoinPool-1-worker-1 starting a new test. Running: 8
ForkJoinPool-1-worker-1 starting a new test. Running: 9
ForkJoinPool-1-worker-2 starting a new test. Running: 10
ForkJoinPool-1-worker-1 starting a new test. Running: 11
ForkJoinPool-1-worker-1 starting a new test. Running: 12
ForkJoinPool-1-worker-1 starting a new test. Running: 14
ForkJoinPool-1-worker-2 starting a new test. Running: 13
ForkJoinPool-1-worker-1 starting a new test. Running: 15
ForkJoinPool-1-worker-1 starting a new test. Running: 16
ForkJoinPool-1-worker-1 starting a new test. Running: 17
ForkJoinPool-1-worker-1 starting a new test. Running: 18
ForkJoinPool-1-worker-2 starting a new test. Running: 19
ForkJoinPool-1-worker-1 starting a new test. Running: 20

worker-1 started 15 tests, worker-2 started 5 tests

In actual code, given the location of the point of respawn, this can generate very large stacks.

An alternative implementation of the executor service, as shown in this PR, using a standard Thread Executor, would not show similar issues, at the expense of not ideally orchestrating multiple executions.

Let's note that this is a very sneaky issue. Even in this project, we can see that the call to ForkJoinTask#get is not visible in the JUnit method. And it can be as deep as we want in the stack, even hidden to some users as it is happening in a used framework or tool. It may not be always possible for a user to detect that pattern.

Steps to reproduce

See this project https://github.com/OPeyrusse/junitforkjoinpool After building the project, run the test class TestCalculator (or look at the README if changed since opening this issue).

Context

Deliverables

igogoka commented 8 months ago
class FixJunit3108Extension implements IGlobalExtension {
    private static ExecutorService threadPool

    private final RunnerConfiguration runnerConfiguration

    FixJunit3108Extension(RunnerConfiguration runnerConfiguration) {
        this.runnerConfiguration = runnerConfiguration
    }

    @Override
    void start() {
        threadPool = Executors.newFixedThreadPool(runnerConfiguration.parallel.parallelExecutionConfiguration.parallelism)
    }

    @Override
    void visitSpec(SpecInfo spec) {
        // work-around for https://github.com/junit-team/junit5/issues/3108
        spec.allFeatures*.addIterationInterceptor {
            threadPool.submit(it::proceed).get()
        }
    }

    @Override
    void stop() {
        threadPool?.shutdown()
    }
}

Thanks. I use your solution, but in Ui tests with Geb and Spock I have problem, that every feature (test in Spec)create new thread. How I can rewrite this method to create a new thread only for Spec ?

 @Override
    void visitSpec(SpecInfo spec) {
        spec.allFeatures*.addIterationInterceptor {
            threadPool.submit(it::proceed).get()
        }

    } 
Vampire commented 8 months ago

It does not create a new thread for every features. It uses a thread pool with a fixed size of which the threads are reused.

igogoka commented 8 months ago

image How I can set defaultExecutionMode = SAME_THREAD, beause it doesn't work from SpockConfig

Vampire commented 8 months ago

You are even more deviating from the topic here. This is not a Spock-support channel and for every message you write, every watcher gets an e-mail. ;-)

mufasa1 commented 8 months ago

@Vampire

This is for Spock. I described above how you probably can do the same for Jupiter.

Hi, I tried the description your wrote, but it didn't work. I would greatly appreciate it if you had a validated solution..

Vampire commented 8 months ago

I provided a full-blown example above, what more do you need?

mufasa1 commented 8 months ago

I provided a full-blown example above, what more do you need?

I mean for Junit, not for Spock. I tried your description like this '@Override public void interceptTestMethod(Invocation invocation, ReflectiveInvocationContext invocationContext, ExtensionContext extensionContext) throws Throwable { threadPool.submit(invocation::proceed).get(); }'

but it didn't work. What I need is a solution that has been validated.

Vampire commented 8 months ago

Works perfectly fine for me, except that in Jupiter proceed throws an exception, so you have to handle it. But once you made the code compilable and ensured it is used, it does exactly what is intended.