[SUREFIRE-1405] Allows user to extend RunOrder & RunOrderCalculator

[dipak-pawar]

• All supported runOrder options are hardCoded in RunOrder - https://github.com/apache/maven-surefire/blob/master/surefire-api/src/main/java/org/apache/maven/surefire/util/RunOrder.java#L109
• With this implementation user can implement his own runOrder & it's respective RunOrderCalculator.

Fixes: https://issues.apache.org/jira/browse/SUREFIRE-1405

Implementation Details

How to create new RunOrder?
To create new runOrder user has to implement RunOrder interface.

public class YourRunOrder implements RunOrder
{

    @Override
    public String getName()
    {
        return "runOrderName";
    }

    @Override
    public List<Class<?>> orderTestClasses( Collection<Class<?>> scannedClasses,
                                            RunOrderParameters runOrderParameters, int threadCount )
    {
       // Add logic to order test classes for this runOrder.
        return testClasses;
    }
}

How to define custom RunOrderCalculator?
Implement RunOrderCalculator interface with logic you want to order test classes.
e.g.

public class YourRunOrderCalculator implements RunOrderCalculator {

    private final RunOrderParameters runOrderParameters;
    private final int threadCount;

    private final RunOrder[] runOrder;

    public YourRunOrderCalculator(RunOrderParameters runOrderParameters, int threadCount) {
        this.runOrder = runOrderParameters.getRunOrders();
        this.runOrderParameters = runOrderParameters;
        this.threadCount = threadCount;
    }
    
    // Add your logic to order test classes.
    @Override 
    public TestsToRun orderTestClasses(TestsToRun scannedClasses) {
      
        List<Class<?>> testClasses = new ArrayList<Class<?>>( 512 );

        for ( Class<?> scannedClass : scannedClasses )
        {
            testClasses.add( scannedClass );
        }

       final Collection<Class<?>> classes = runOrder[0].orderTestClasses(testClasses, runOrderParameters, threadCount);

        return new TestsToRun(new LinkedHashSet<>(classes));
    }
}

How to tell surefire to use custom RunOrder & RunOrderCalculator?

We have RunOrderProvider spi to overwrite default runOrders & RunOrderCalculator provided by surefire.

You need to registrar impl of RunOrderProvider in the file named
META-INF/services/org.apache.maven.plugin.surefire.runorder.spi.RunOrderProvider in main resources.

File should contain fully qualified name of RunOrderProvider impl.
e.g.
com.surefire.YourRunOrderProviderImpl

Implementation of YourRunOrderProviderImpl is as follows:

public class YourRunOrderProviderImpl implements RunOrderProvider
{

    @Override
    public Collection<RunOrder> getRunOrders()
    {
   // here you can give all default runorders provided by surefire along with custom runorder created above.
        return Arrays.asList( ALPHABETICAL, FILESYSTEM, HOURLY,
            RANDOM, REVERSE_ALPHABETICAL, BALANCED, FAILEDFIRST, new YourRunOrder() );
    }

    @Override
    public Integer priority()
    {
        return 1;
    }

    @Override
    public RunOrderCalculator createRunOrderCalculator( RunOrderParameters runOrderParameters, int threadCount )
    {
        return new YourRunOrderCalculator( runOrderParameters, threadCount );
    }

    @Override
    public Collection<RunOrder> defaultRunOrder()
    {
        return Collections.singletonList( FILESYSTEM );
    }
}

[Tibor17]

Thx for your effort but we have to wait for this till the version 3.0.
The branch will have more merge conflicts. Please rebase this in few months when we will be about develop 3.0.

[dipak-pawar]

Sure, we will be happy to move it forward. Please keep this PR open and let us know by commenting here when it will make sense to resume the effort. I really hope we can make it upstream.

[famod]

@dipak-pawar Any chance to resurrect this?

@Tibor17 Or is something different planned for the next milestone(s)?

[Tibor17]

@dipak-pawar Any chance to resurrect this?

@Tibor17 Or is something different planned for the next milestone(s)?

No, not at all, but we have to dig into this again and review this. That's the whole problem.

[famod]

@Tibor17 I noticed that JUnit 5 will have some kind test class ordering in 5.8.0: junit-team/junit5#2488

I'm wondering whether this is going to just work with surefire?
Do we still need something like this PR here for ordering on the surefire side?

[Tibor17]

@Tibor17 I noticed that JUnit 5 will have some kind test class ordering in 5.8.0: junit-team/junit5#2488

I'm wondering whether this is going to just work with surefire?
Do we still need something like this PR here for ordering on the surefire side?

@famod
This is very good patch from @dipak-pawar and I will try to rebase it on the latest source code in master. But this has nothing to do with the JUnit5. They do their job and we do ours. We must have the ability to order the classes. How can you otherwise order the classes while multiple forks wants to execute the tests? You cannot do it in JUnit5. You have to do it before the fork starter.

So this was wrong question. You must see the pipeline in order to understand what config parameter and how to use.

[famod]

@Tibor17

How can you otherwise order the classes while multiple forks wants to execute the tests?

Ok, thanks for confirming that this is still the case for JUnit5. I am well aware of this implication from previous projects using JUnit4 but I wasn't sure the same applies to the way surefire handles JUnit5.

This also applies to single-fork mode (the default), right?

[Tibor17]

@Tibor17

How can you otherwise order the classes while multiple forks wants to execute the tests?

Ok, thanks for confirming that this is still the case for JUnit5. I am well aware of this implication from previous projects using JUnit4 but I wasn't sure the same applies to the way surefire handles JUnit5.

This also applies to single-fork mode (the default), right?

@famod
This PR is an excellent solution where the old implementation has got reworked using the abstraction and polymorphism which opens the Extensions API for our users to reimplement it without asking the ASF team. So this PR should not be seen as a fix nor feature for JUnit5. The Extensions API was long lasting plan for us nevertheless JUnit5 was released.
Regarding your question about a single JVM execution, the run-order in junit may be used instead. The surefire's run-order is always used by the plugin. I do not remember what type of run-order is default but it is mentioned in the documentation for the configuration parameter.

[Tibor17]

@dipak-pawar
You made an excellent job! I would like to continue on the review with you. I would be happy if you agree to continue on this PR.
We have also moved forward with the project after 3 years. For instance we have a new module with the extensions, some conventions regarding the SPI and we have not to use the terminology "class" nothing but some generic form (e.g. a script file too).

[aslakhellesoy]

@Tibor17 I'm interested in helping out to move this forward.

Given that the master branch has moved on significantly since this @dipak-pawar submitted this PR, I think it would be easier to create a new pull request from the master branch, and manually bring in the changes.

Please let me know what you think.

[Tibor17]

@aslakhellesoy
It should be taken very carefully because there is another similar requirement with run order in #348.
It would be nice if you would agree with @winglam on the interface of RunOrder. You both have some requirements. I also have some requirements, after long time, we have implemented extensions which gives the user an opportunity to select the implementation in the POM instead of SPI. The SPI is fine it is is one in the JRA, but if you have many implementations you can select it in the POM without using SPI mechanism. Of course this requires more changes including more work in the surefire serialization.

[Tibor17]

Additionally, we should not use Collection<Class<?>> scannedClasses because the JUnit5 has abstract description of test and they can be also scripts, so the time has changed and this would require us to think of some non-class form of test in terms of abstractions.

[aslakhellesoy]

Thanks for the quick answer @Tibor17. I wasn't aware of #348 - from the first glance it looks like this implementation would address my needs!