Introduce `pleaseStop()` method for `org.junit.platform.launcher.Launcher`

[Tibor17]

We would like to send a command to the engine to control the execution and stop the progress of pending tests via the interface org.junit.platform.launcher.Launcher.

[Anton-nsk]

I'd like that too. I ran into a problem that is related to this and reported to Jira. Please implement this because it would solve my problem. Also, I think I'm not the only one with this.

[mpkorstanje]

I've done a little big of digging to see if it would be possible to implement this. I think it is.

For the purpose of this discussion I'll be using the HierarchicalTestEngine as reference. I'm assuming that other TestEngine implementations have a comparable layout and can apply a similair approach.

The HierarchicalTestEngine comes with a HierarchicalTestExecutorService which is created by invoking createExecutorService with the ExecutionRequest as a parameter.

    protected HierarchicalTestExecutorService createExecutorService(ExecutionRequest request) {
        return new SameThreadHierarchicalTestExecutorService();
    }

The ExecutionRequest contains an EngineExecutionListener.

Because the EngineExecutionListener exists before the HierarchicalTestExecutorService is created it will not be possible to implement a pleaseStop on the HierarchicalTestExecutorService.

However prior to execution the HierarchicalTestExecutorService could ask the EngineExecutionListener if it should execute the next task. So we can implement EngineExecutionListener.canWeContinue().

For example looking at ForkJoinPoolHierarchicalTestExecutorService.ExclusiveTask the implementation would have to do something like:

    static class ExclusiveTask extends RecursiveAction {

        private final TestTask testTask;
                private final EngineExecutionListener listener;
        @Override
        public void compute() {
                        if (!listener.canWeContinue()){
                               testTask.skip(); // This would execute the test task and result in a Skipped status.
                               return;
                        }
            try (ResourceLock lock = testTask.getResourceLock().acquire()) {
                testTask.execute();
            }
            catch (InterruptedException e) {
                ExceptionUtils.throwAsUncheckedException(e);
            }
        }

    }

Naming would obviously have to be improved a bit and I think the canWeContinue might best be placed on it's own separate interface rather then in EngineExecutionListener.

[sbrannen]

@mpkorstanje, thanks for investigating the feasibility of this feature.

This is currently scheduled for the 5.7 backlog, so we hope to look into this during the 5.7 time frame.

[Tibor17]

@sbrannen It is already in 5.6 Backlog. The problem is that we repeat the same bad from JUnit4. Maybe you remember that the listener RunListener contains the method pleaseStop. And the core of the problem is the design because the listener methods are typical event methods to read data, and especially for the reads operations, although the methods canWeContinue and pleaseStop are typical commands for writes. And that's why the guys had a problem with this issue. IMO the solution should go with a new SPI ala Stoppable and not a listener. Then the design will be ok and we break the continuation of the bad from JUnit4.

[Tibor17]

I did not say important fact that introducing an abstract method in a listener would mean breaking the backwards compatibility where the mandatory method should be implemented; otherwise the runtime with higher version of engine would throw NoSuchMethodError.

[sbrannen]

I did not say important fact that introducing an abstract method in a listener would mean breaking the backwards compatibility where the mandatory method should be implemented; otherwise the runtime with higher version of engine would throw NoSuchMethodError.

If we add a new method to an existing type, it would likely be in an interface... in which case we would almost certainly introduce an interface default method. So I don't foresee any major issues with backwards compatibility.

Or did you mean something else?

[Tibor17]

@sbrannen Did you see my previous comment properly? https://github.com/junit-team/junit5/issues/1880#issuecomment-572935329 It is not good to make the listener dirty. Due to the SPI can be easily loaded anywhere, why don't you introduce a new getter default method getStoppable in TestTask . As well as you have another resource method getResourceLock. The default method would return the default Stoppable. The IDEs may easily override the behavior from default to custom just by adding the Stoppable SPI to the classpath. The execution of engine would be isolated from the EngineExecutionListener and it has to because the listener has different purpose. And really the pleaseStop method is not necessarily derived from the status notified by the EngineExecutionListener and no reason to force the user to implement a listener only due to one method and register the listener for orthogonal purpose.

[marcphilipp]

Brainstorming ideas: We discussed a few options on how the Launcher API could be extended:

• We could add a new overload of Launcher.execute() that takes a new object (working title "execution control"), potentially wrapped in a parameter object, that engines should poll (in case they support cancellation) whenever they're at a point where they could abort execution, e.g. before starting a new test.
• Another option would be to add an asynchronous execute() method that would return a Future-like object that has methods to cancel execution. It might have separate methods: one that enforces a shutdown as soon as possible and one that does it gracefully.

[sormuras]

The first idea relates to #90 - perhaps this pleaseStop() feature request could be part of a solution for the "test engine capabilities" support.

[kriegfrj]

Brainstorming ideas: We discussed a few options on how the Launcher API could be extended:

• We could add a new overload of Launcher.execute() that takes a new object (working title "execution control"), potentially wrapped in a parameter object, that engines should poll (in case they support cancellation) whenever they're at a point where they could abort execution, e.g. before starting a new test.
• Another option would be to add an asynchronous execute() method that would return a Future-like object that has methods to cancel execution. It might have separate methods: one that enforces a shutdown as soon as possible and one that does it gracefully.

Interesting that in my duplicate issue #2462, I came up with the same idea as the last one - to have an asynchronous execute() method that returns a future-like object. The more I think about it, the more I think that this is the cleanest way to go.

[oehme]

This would be great for Gradle users on Windows. Right now, if you cancel a Gradle build, Gradle will shut down the test process, because there is no better API for stopping tests. On Linux/Mac this is a graceful shutdown, so you can clean things up in your tests with shutdown handlers. On Windows, the shutdown is immediate and no shutdown handlers get executed. As a result, your tests won't clean up after themselves.

If we had this API, Gradle could ask it to stop this way first, allowing tests to clean up any services they started etc.

[stale[bot]]

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[bot]]

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.

[Vampire]

Any news on this one? :-)

[uchagani]

Is there anything the community can help with this?

[renyiwei-xinyi]

Looking forward to the conclusion and updates on this issue~

[marcphilipp]

As a next step, we should explore the ideas from https://github.com/junit-team/junit5/issues/1880#issuecomment-575120923 further. I'm all for it if anyone wants to take a stab at this. Please be aware that it's exploratory work, though, and we might end up not merging the PR.