cucumber / cucumber-jvm

Cucumber for the JVM
https://cucumber.io
MIT License
2.7k stars 2.02k forks source link

There is no proper saturation when running bdd tests in parallel by scenarios #2754

Closed p-nikolaichik closed 1 year ago

p-nikolaichik commented 1 year ago

👓 What did you see?

I run 1000 scenarios by 60 threads with following junit-platform.properties:

cucumber.execution.parallel.enabled=true cucumber.execution.parallel.config.strategy=fixed cucumber.execution.parallel.config.fixed.parallelism=60 cucumber.execution.parallel.config.fixed.max-pool-size=60 cucumber.plugin=io.cucumber.core.plugin.SerenityReporterParallel cucumber.filter.tags=not @skip and not @manual

The number of active threads is not keeping at 60, new threads start only when most of the active threads (at least half) are finished. The graphs specified below show that saturation occurs in batches, when near 30-50 threads are completed only after that next threads are run. Other words there is no proper saturation of specified number of threads, therefore the time of execution is in 2 times more than if I run the same test suite in parallel by feature files using junit4 with cucumber-jvm-parallel-plugin

✅ What did you expect to see?

Thread pool should be saturated constantly (not only when half of tests are finished), if at least one test is finished another test should start.

Environment: (serenity + junit5 + cucumber7), is reproduced with java 11 and java 17

    <serenity.version>3.7.1</serenity.version>
    <junit.platform.version>1.9.3</junit.platform.version>
    <cucumber.junit.platform.engine.version>7.11.2</cucumber.junit.platform.engine.version>
    <junit.jupiter.migration.support>5.0.0-M4</junit.jupiter.migration.support>

    <dependency>
        <groupId>net.serenity-bdd</groupId>
        <artifactId>serenity-cucumber</artifactId>
        <version>${serenity.version}</version>
    </dependency>
    <dependency>
        <groupId>net.serenity-bdd</groupId>
        <artifactId>serenity-junit5</artifactId>
        <version>${serenity.version}</version>
    </dependency>
    <dependency>
        <groupId>io.cucumber</groupId>
        <artifactId>cucumber-junit-platform-engine</artifactId>
        <version>${cucumber.junit.platform.engine.version}</version>
    </dependency>
    <dependency>
        <groupId>org.junit.platform</groupId>
        <artifactId>junit-platform-suite</artifactId>
        <version>${junit.platform.version}</version>
    </dependency>
    <dependency>
        <groupId>org.junit.jupiter</groupId>
        <artifactId>junit-jupiter-migration-support</artifactId>
        <version>${junit.jupiter.migration.support}</version>
    </dependency>

image image


mpkorstanje commented 1 year ago

Nice graphs!

When using the cucumber-junit-platform-engine the parallel execution is facilitated by the ForkJoinPoolHierarchicalTestExecutorService from JUnit 5. So ultimately this means that any fix would have to be submitted to JUnit 5.

Before we go there, would you happen to have a minimal reproducer that you can share in the form of a Github repository?

Now I suspect that you're running into this specific comment:

        // Limit the amount of queued work so we don't consume dynamic tests too eagerly
        // by forking only if the current worker thread's queue length is below the
        // desired parallelism. This optimistically assumes that the already queued tasks
        // can be stolen by other workers and the new task requires about the same
        // execution time as the already queued tasks. If the other workers are busy,
        // the parallelism is already at its desired level. If all already queued tasks
        // can be stolen by otherwise idle workers and the new task takes significantly
        // longer, parallelism will drop. However, that only happens if the enclosing test
        // task is the only one remaining which should rarely be the case.
        if (testTask.getExecutionMode() == CONCURRENT && ForkJoinTask.getSurplusQueuedTaskCount() < parallelism) {
            return exclusiveTask.fork();
        }
        exclusiveTask.compute();
        return completedFuture(null);

But a reproducer would help to verify that and potentially make a stronger argument for changing this behavior.

p-nikolaichik commented 1 year ago

I decided to compare run using selenium instead of serenity to be totally sure and saw that selenium with junit5 and cucumber doesn't have such problems. I created ticket in serenity project: https://github.com/serenity-bdd/serenity-core/issues/3202 and attached simple project reproducer there

mpkorstanje commented 1 year ago

Cheers! Looks like the problem is caused by Serenity then. Should it still turnout to be a problem with Cucumber and JUnit 5 please do feel free to open a more specific issue.