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

Parallelization control of DynamicNode #2497

Open
big-andy-coates opened this issue Dec 18, 2020 · 16 comments
Open

Parallelization control of DynamicNode #2497

big-andy-coates opened this issue Dec 18, 2020 · 16 comments

Comments

@big-andy-coates
Copy link

@TestFactory can return a stream of DynamicContainers and DynamicTest. It would be awesome if we could run containers in parallel, but the tests within containers on the same thread.

The @Execution seems to only allow you to switch from everything parallel/concurrent or everything on the same thread.

The configuration parameters junit.jupiter.execution.parallel.mode.default and junit.jupiter.execution.parallel.mode.classes.default also don't seem to allow such a configuration, probably because all test suites and test are coming from the same class.

For example: the following should allow tests test1 and test2 that are in suite1 to be run on the same thread, and the tests in suite2 to be run on a different, but consistent thread:

static class Runner {

    @TestFactory
    Stream<DynamicNode> systemTest() {
        final Stream<DynamicNode> tests1 = Stream.of(
                        DynamicTest.dynamicTest("test1", () -> {System.out.println(Thread.currentThread().getId() + " -test1");}),
                        DynamicTest.dynamicTest("test2", () -> {System.out.println(Thread.currentThread().getId() + " - test2");}));

        final Stream<DynamicNode> tests2 = Stream.of(
                        DynamicTest.dynamicTest("test3", () -> {System.out.println(Thread.currentThread().getId() + " -test3");}),
                        DynamicTest.dynamicTest("test4", () -> {System.out.println(Thread.currentThread().getId() + " - test4");}));

        return Stream.of(
                DynamicContainer.dynamicContainer("suite1", tests1),
                DynamicContainer.dynamicContainer("suite2", tests2));
    }
}

Maybe this could be achieved via one or more of:

  • a new junit.jupiter.execution.parallel.mode.container.default property
  • a field on @TestFactory to control execution
  • an enhancement to @Execution
  • a parameter to DynamicContainer

Thanks!

@marcphilipp
Copy link
Member

Tentatively slated for 5.8 M1 solely for the purpose of team discussion.

@sic-poths
Copy link

We have a similar case, but would like to run the first level of dynamic nodes in sequence, but tests under each level can be run in parallel. It would be great to be able to define the ExecutionMode on each DynamicContainer instance.

@marcphilipp marcphilipp modified the milestones: 5.8 M1, 5.8 M2/RC1 Feb 7, 2021
@marcphilipp marcphilipp removed this from the 5.8 RC1 milestone Aug 15, 2021
@stale
Copy link

stale bot commented Sep 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label Sep 16, 2022
@sic-poths
Copy link

I would still be really interested in a solution to this problem. Are there any thoughts from the team on what a good implementation of a more fine granular "execution-order-control" for dynamic tests could be? Maybe I would give it a try to create a PR, but I only know the codebase as a user.

@stale stale bot removed the status: stale label Sep 16, 2022
@niteshhardikar
Copy link

Perfectly explained by @big-andy-coates. I have run into the same requirement where I want to run my DynamicContainers in parallel but the DynamicTests within each container sequentially. Is this slated for any upcoming release or is there any other solution to this problem for instance using the Extension model? Any inputs are highly appreciated.

@marcphilipp
Copy link
Member

I wonder if a configuration parameter would really be adequate here. Wouldn't it be better if DynamicNode would provide an API to configure the execution mode similiar to @Execution for regular test classes and methods?

@big-andy-coates @sic-poths @niteshhardikar WDYT?

@big-andy-coates
Copy link
Author

Yep, that's certainly one option. I mentioned in the original text about adding "a parameter to DynamicContainer", though as you note, DynamicNode would be better.

@niteshhardikar
Copy link

Yes @marcphilipp. Supporting configuration of execution mode for DynamicNode would be ideal.

@marcphilipp
Copy link
Member

@big-andy-coates @niteshhardikar Do you have concrete proposals for that API? I'm reluctant to add more overloads for DynamicTest.dynamicTest and DynamicContainer.dynamicContainer. 🤔

@big-andy-coates
Copy link
Author

big-andy-coates commented Apr 23, 2023

Maybe it's time to introduce a builder pattern, rather than overloading?

DynamicContainer.builder("displayName")
   .withNodes(iterable)
   .withNodes(stream)
   .withTestSource(uri)
   .withExecutionMode(mode)
   .build();

...and deprecate the old factory methods for removal at the next major version bump.

@marcphilipp
Copy link
Member

The methods in this codebase don't use the with prefix (see LauncherDiscoveryRequestBuilder). Apart from that I can see an API like that work.

This recent PR is also related: #3220 (review)

@marcphilipp
Copy link
Member

Team decision: While we think this is a reasonable request, we'd like to wait for additional interest from the community

@novoj
Copy link

novoj commented Jun 30, 2023

What can the interested community do to move this forward? Are you waiting for a certain number of votes/responses here or how "the interest" is measured?

@marcphilipp
Copy link
Member

Yes, interest is measured in the number of upvotes.

@prathasirisha
Copy link

I have upvoted 👍🏼 for the issue and the use case outlined here: #2497 (comment)

@sonthanh
Copy link

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants