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

Introduce mechanism for terminating Dynamic Tests early #431

Open
mmerdes opened this issue Jul 25, 2016 · 13 comments
Open

Introduce mechanism for terminating Dynamic Tests early #431

mmerdes opened this issue Jul 25, 2016 · 13 comments

Comments

@mmerdes
Copy link
Contributor

mmerdes commented Jul 25, 2016

Dynamic Tests are non-deterministic in nature. Sometimes it might not be clear to the test author (let alone the IDE) when or even if the execution will terminate.
It has been suggested to introduce a timeout to stop the execution before it would terminate by itself. An alternative would be to terminate the execution when a certain number of failed (or successful tests) have been executed.

@sormuras
Copy link
Member

sormuras commented May 6, 2017

With #838 in the works, it is easy to get and store the initial time instant when a "@testfactory"-annotated methods started. The user-supplied code then could calculate the difference to now and throw an (time out) exception in a "requiredTest" executable. That breaks the execution loop.

Or, a time-out value could be passed at the "@testfactory" annotaion level and the interpreted internally.

@sormuras
Copy link
Member

sormuras commented May 6, 2017

Or the user may supply a predicate object that is asked before every dynamic test execution whether the execution loop should continue or not.

@sormuras
Copy link
Member

sormuras commented May 6, 2017

The time-out use-case is already achievable via:

@TestFactory
Stream<DynamicNode> dynamicTestsWithContainers() {
    Instant start = Instant.now();
    return Stream.of("A", "B", "C")
      .map(input -> dynamicContainer("Container " + input, Stream.of(
        dynamicTest("not null", () -> assertNotNull(input)),
        requiredTest("check duration",
            () -> {
              long duration = Duration.between(start, Instant.now()).toMillis();
              if (duration > 23) {
                fail("Time is over: " + duration + " ms");
              }
            }
        ),
        dynamicContainer("properties", Stream.of(
          dynamicTest("length > 0", () -> assertTrue(input.length() > 0)),
          dynamicTest("not empty", () -> assertFalse(input.isEmpty()))
        ))
      )));
}

@sormuras
Copy link
Member

sormuras commented May 6, 2017

tl;dr #838 will solve this issue.

dynamictest-required-with-timeout

@sbrannen
Copy link
Member

sbrannen commented May 7, 2017

Using a "required test" that throws an exception seems to be a hack to me.

I definitely would not call that intuitive in terms of the programming model.

Similarly, consider what the user's code would look like when implementing a custom Iterable instead of returning a stream. At some point the iterable would have to return a "required test" that only throws an exception, and that just seems wrong to me.

IMHO, I believe we need a more elegant, first-class solution to the problem addressed by this issue -- one that works even if there is no such thing as a "required test".

@sbrannen
Copy link
Member

sbrannen commented May 7, 2017

Supporting a user-defined timeout per test factory and/or a predicate that is provided a contextual object (containing information such as number of successful test, number of failed tests, number of aborted tests, number of skipped tests, duration of execution thus far, etc.) would be more intuitive and less burdensome on the end user.

@sormuras
Copy link
Member

sormuras commented May 8, 2017

The current iteration of #838 supports a DynamicTest.dynamicTest() overload that takes an user-provided Predicat<Boolean> that is evaluated after the executable ran. The Boolean parameter can easily replaced with the contextual object @sbrannen mentions above.

Done in b5c514a

@nipafx
Copy link
Contributor

nipafx commented May 9, 2017

As far as I know the dynamic test API makes no promise about when the tests will be executed - it is possible to change the implementation so that the factory method is called and the dynamic tests are executed much later. I think this is a good thing and maybe should even be documented.

If you share that view, any implementation of this feature should make sure to not accidentally rely on the immediacy of the test execution, thus locking JUnit into that behavior. (I realized all of this when looking at your idea for how to implement the time out.)

@sormuras sormuras modified the milestones: 5.0 M6, 5.0 M5 May 9, 2017
@sormuras
Copy link
Member

sormuras commented May 9, 2017

Too many concerns and open ends with #838

"we should think it over more and brainstorm about the user experience and programming model before doing any actual coding. In other words, we need to design the feature first."

@sormuras sormuras modified the milestones: 5.1 Backlog, 5.0 M6 Jul 10, 2017
@sormuras sormuras removed their assignment Aug 15, 2018
@dmitry-timofeev
Copy link
Contributor

I bumped into this issue when attempted to build a prototype of early termination of tests in a container once a failed one is discovered in pitest-junit5-plugin.

Pitest, a mutation coverage tool, runs tests against a mutation of the code they cover to determine if these tests are good enough — if they detect mutants. When it runs a parameterized or dynamic JUnit 5 test and detects a test failure, there is usually no need to run subsequent tests — but that is controlled by Pitest configuration.

The workaround I used involved an extension implementing ExecutionCondition that filters tests once a failure is detected. But as Pitest controls if an individual test failure must abort test execution, and JUnit Jupiter Engine instantiates the extensions, the extension has to use a global static object to communicate with the rest of the system. Can't say if it is a generally good idea, but in the case of that plugin a feature allowing to abort the execution of subsequent tests at platform level seems to be useful, because no extra extensions and communication through globals would be needed.

@stale
Copy link

stale bot commented May 13, 2021

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 May 13, 2021
@stale
Copy link

stale bot commented Jun 3, 2021

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

@stale stale bot closed this as completed Jun 3, 2021
@Captain-P-Goldfish
Copy link

I would like to have this issue reopened.
I think I got a pretty good usecase for aborting the execution of dynamic tests:

I am currently writing selenium integration tests and I am using DynamicTests for this purpose to create a logical dynamic workflow in the test. The problem here is if even one of the steps fails it is to be assumed that the following tests will fail too. So it would be great if the @TestFactory annotation could be extended by an additional parameter abortOnFail or something similiar that stops the execution of additional tests if somewhere along the road a test failed

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