Junit5: 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.