You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/04/16 18:30:07 UTC

[GitHub] [maven-surefire] winglam opened a new pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a runOrder

winglam opened a new pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348


   The changes in this pull request enable Surefire to run test classes and test methods in any order specified by a newly added [\<runOrder\>](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#runOrder) (testorder). Namely, the changes include
   - A new [\<runOrder\>](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#runOrder) called testorder that would make Surefire run test classes and test methods in the order specified by [\<test\>](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#test). Regex and include/exclude are supported as is the case for other runOrders
   - Tests to TestListResolverTest and RunOrderCalculatorTest for the newly added features
    
   The newly added testorder [\<runOrder\>](http://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#runOrder) is supported for all JUnit 3 and JUnit 4 tests. I will plan on supporting JUnit 5 tests after these changes are accepted.
   
   **Simple (no regex) example using [Dubbo](https://github.com/apache/dubbo.git):**
   ```
   mvn test -Dsurefire.runOrder=testorder \
   -Dtest=org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest#testSticky2,\
   org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest#testSticky1,\
   org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest#testSticky3,\
   org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest#testNonSerializedParameter\
    -pl dubbo-rpc/dubbo-rpc-dubbo
   ```
    
   Output from Maven should say that the test class `DubboLazyConnectTest` ran before `DubboProtocolTest`
    
   The file `DubboLazyConnectTest.xml` (`dubbo-rpc/dubbo-rpc-dubbo/target/surefire-reports/TEST-org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest.xml`) should say 
   ```
   <testcase name="testSticky2"...>
   <testcase name="testSticky1"...>
   <testcase name="testSticky3...>
   ```
    
   **Regex example using [Dubbo](https://github.com/apache/dubbo.git):**
   ```
   mvn test -Dsurefire.runOrder=testorder \
   -Dtest=org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest*,\
   org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest#testDubboProtocol*\
    -pl dubbo-rpc/dubbo-rpc-dubbo
   ```
    
   Output from Maven should say 
   ```
   Tests run: 4 … in org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest
   Tests run: 3 … in org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest
   ```
   
   The file `DubboProtocolTest.xml` (`dubbo-rpc/dubbo-rpc-dubbo/target/surefire-reports/TEST-org.apache.dubbo.rpc.protocol.dubbo.DubboProtocolTest.xml`) should say 
   ```
   <testcase name=”testDubboProtocol”...>
   <testcase name="testDubboProtocolWithMina"...>
   <testcase name="testDubboProtocolMultiService”...>
   ```
    
   **Include/Exclude example using [Dubbo](https://github.com/apache/dubbo.git):**
   ```
   mvn test -Dsurefire.runOrder=testorder \
   -Dtest=org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest#test*,\
   !org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest#testSticky2\
    -pl dubbo-rpc/dubbo-rpc-dubbo
   ```
   
   Output from Maven should say 
   `Tests run: 3 … in org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest`
   
   The file `DubboLazyConnectTest.xml` (`dubbo-rpc/dubbo-rpc-dubbo/target/surefire-reports/TEST-org.apache.dubbo.rpc.protocol.dubbo.DubboLazyConnectTest.xml`) should say 
   ```
   <testcase name="testSticky1"...>
   <testcase name="testSticky3"...>
   <testcase name="testSticky4...>
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1070857800


   @aslakhellesoy
   The code is not suited to rebase because this code has some issues. That's the reason why I wanted to save the spare time of @winglam and I wrote 5 steps on how to implement the feature right way and fast enough, see the comments written since of January 27 2022.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022747001


   Maybe a hint, try to run a basic project in debug mode `mvn -X test -Dtests=xxx,yyy` and go to the folder `target/surefire`. You should see two properties files and one jar file. Open one property file and you should see CSV of what you have used in `-Dtest=`. This is your data. Iterate over this parse each substring, create an instance of `TestListResolver` and pickup the class/method pairs. Order them and use them as necesary in the run order calculator within the provider(s).
   Regarding the sanity check, go to the `AbstractSurefireMojo` class and see the parameters checkers in the `execute()` method. There are more like this. I think you should write a check that the new enum requires the test list but do not use `getTests()` directly. We are merging several config parameters together and we will change this part of code in #440 so my advice is to focus on this a bit later or check the #440 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077771434


   @aslakhellesoy Thanks for the clarification. I was aware from the https://issues.apache.org/jira/browse/SUREFIRE-2041 issue that you wanted a global test method ordering feature that supports the interleaving of test methods across test classes and I agree that it may not be realistic to attempt. 
   
   More importantly, I'm not sure how one would implement such a feature. Namely, how should one handle class setup and teardown methods (e.g., @BeforeClass from JUnit) for an order such as testing.Y#a,testing.X#c,testing.Y#b? Should one run _testing.Y#beforeClass_,testing.Y#a,_testing.X#beforeClass_,testing.X#c,_testing.Y#beforeClass_,testing.Y#b? What if running testing.Y#beforeClass twice without running testing.Y#afterClass causes a failure? We could then run _testing.Y#afterClass_ after testing.Y#a and before _testing.X#beforeClass_ but then that can add substantial time cost to running tests and still cause unexpected failures if these class setup and teardown methods are not expected to run twice in one JVM.
   
   Not supporting a global test method ordering feature is a purposeful design feature when I did my implementation of test method ordering. For the same reason we may not want to support the feature of interleaving test classes across different modules (e.g., module1.testing.Y,module2.testing.X,module1.testing.Z), I'm not sure whether a global test method ordering feature should be supported or not based on the way modules and test classes are defined and commonly used. I believe there is still much value to support test method ordering within classes (and also the ordering of test classes) and the feature I have in this PR accomplishes both test class ordering and test method ordering, just not the interleaving of test methods across classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1021683915


   Hi @winglam
   What's up?
   
   I checked your commit.
   I have one argument to the changes in `TestListResolver`. This class is a filter and not a Comparator. It would be worth not to change it. Are you aiming for comparing two (class, method) pairs/objects? Would it be possible with a separate class? Do you still need to have `TestListResolver`? I guess you don't because the tests which have run before, they would be a subject to run again. The only trick is to reorder these pairs, am I right?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022720153


   The `TestListResolver` does not do the trick. It is only a filter. You should not use it.
   The whole trick is done in every Surefire Provider implementation. So in this case, you have to modify the providers.
   I think this snap code explains how we do it, see the `JUnit4Provider`:
   ```
       private TestsToRun scanClassPath()
       {
           final TestsToRun scannedClasses = scanResult.applyFilter( jUnit4TestChecker, testClassLoader );
           return runOrderCalculator.orderTestClasses( scannedClasses );
       }
   ```
   The problem is that we do not work with methods. Only classes.
   And we should remember JUnit5 because it knows the scripts, dynamic tests, directories, etc.
   
   So all you have to do is to serialize run order (enum + data), see `BooterSerializer` and deserialize it via `BooterDeserializer`.
   It means that you have transfered the config for `RunOrderCalculator` fom plugin JVM to the forked JVM. Similar serialization trick in the in-plugin JVM.
   Then you should obtain the instance of `RunOrderCalculator` in the particular provider. The next step is to modify the section I mentioned above which would include the methods as well.
   
   The system property and TestListResolver parts can be avoided.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022747001


   Maybe a hint, try to run a basic project in debug mode `mvn -X test -Dtests=xxx,yyy` and go to the folder `target/surefire`. You should see two properties files and one jar file. Open one property file and you should see CSV of what you have used in `-Dtest=`. This is your data. Iterate over this parse each substring, create an instance of `TestListResolver` and pickup the class/method pairs. Order them and use them as necesary in the run order calculator within the provider(s), see the `ProviderParameters#getTestRequest()`.
   Regarding the sanity check, go to the `AbstractSurefireMojo` class and see the parameters checkers in the `execute()` method. There are more like this. I think you should write a check that the new enum requires the test list but do not use `getTests()` directly. We are merging several config parameters together and we will change this part of code in #440 so my advice is to focus on this a bit later or check the #440 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] aslakhellesoy commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
aslakhellesoy commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1073805631


   Hi @winglam - I have created a JIRA issue related to this: https://issues.apache.org/jira/browse/SUREFIRE-2041
   
   In that JIRA issue I have outlined why I don't think ordering test methods is possible (only classes). I'd be interested in your thoughts about this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1074266300


   @aslakhellesoy as the very first message in this PR explains already, the capability for Surefire to order test methods is already implemented -- albeit implemented in a way that Tibor would prefer that we change to merge the feature into Surefire.
   
   Mainly we just need to follow the steps Tibor outlined so that Surefire can order not just classes but also methods. Happy to comment on the work or test it out, but I won't have time to implement it myself for the next few weeks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1072429832


   @Tibor17 and @aslakhellesoy I would be happy to participate in the discussion for this feature. Unfortunately, I would not have time to implement the five-step process that Tibor laid out by myself until this May. 
   
   @aslakhellesoy If you are able to implement the five steps before then, please feel free to do so. I would be happy to look over a PR and work with you to get this feature out the door. Please let me know if this option interests you and how may I help.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077876686


   I am not actively working on a test prioritization engine at the moment but have some work on it in the past, e.g., https://cs.gmu.edu/~winglam/publications/2020/LamETAL20ISSTA.pdf 
   
   I agree with your Twitter post that we should ideally be able to run them in any order (even orders that interleave) but as you pointed out, such a feature should probably first come from JUnit before it gets to Surefire.
   
   I also agree that it is still possible to do TCP with class ordering, but why stop at class ordering? Why not have your TCP be done at class + non-interleaving method ordering? As we've discussed, global test ordering or interleaving method ordering is going to require substantial efforts to accomplish, but class + non-interleaving method ordering is already supported in this PR and just requires one to improve this PR according to the suggestions Tibor provided.
   
   Thanks for the heads up about Cucumber. I was not aware that the framework supported global test ordering. Do they run @BeforeClass (or their version of it) once before the first test even if methods from different classes are interleaved?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077771434


   @aslakhellesoy Thanks for the clarification. I was aware from the https://issues.apache.org/jira/browse/SUREFIRE-2041 issue that you wanted a global test method ordering feature that supports the interleaving of test methods across test classes and I agree that it may not be realistic to attempt. 
   
   More importantly, I'm not sure how one would implement such a feature. Namely, how should one handle class setup and teardown methods (e.g., @BeforeClass from JUnit) for an order such as testing.Y#a,testing.X#c,testing.Y#b? Should one run _testing.Y#beforeClass_,testing.Y#a,_testing.X#beforeClass_,testing.X#c,_testing.Y#beforeClass_,testing.Y#b? What if running testing.Y#beforeClass twice without running testing.Y#afterClass causes a failure? We could then run _testing.Y#afterClass_ after testing.Y#a and before _testing.X#beforeClass_ but then that can add substantial time cost to running tests and still cause unexpected failures if these class setup and teardown methods are not expected to run twice in one JVM.
   
   Not supporting a global test method ordering feature is a purposeful design feature when I did my implementation of test method ordering. For the same reason we may not want to support the feature of interleaving test classes across different modules (e.g., module1.testing.Y,module2.testing.X,module1.testing.Z), I'm not sure whether a global test method ordering feature should be supported or not based on the way modules and test classes are defined and commonly used. On the other hand, just like how we support test class ordering within modules, I believe there is still much value to support test method ordering within classes (and also the ordering of test classes). The feature I have in this PR accomplishes both test class ordering and test method ordering, just not the interleaving of test methods across classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022581902


   Hello @Tibor17 
   
   Thanks for following up on these changes. I'm a bit busy this week but I will plan on updating these changes by next week.
   
   Currently, my changes seem to use `TestListResolver` as both a filter and a Comparator (https://github.com/apache/maven-surefire/pull/348/files#diff-390b26f9fa7b88f45e645b6b1fc6b4018570ea482062b93ea85fa00008c1fe20R533). 
   Would it be fine if I keep the filter-related changes in `TestListResolver` and just moved the Comparator changes elsewhere, or do you want me to not make any changes to `TestListResolver` at all?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022724116


   It would help you and us if you follow these steps, in separate commits, and each step would have unit tests:
   1. serialize run order config + test
   2. deserialize config + test
   3. create deserialized instance of run order calculator, verify the config, and write a test
   4. adjust the provider, isolate it in a test
   5. write an integration test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1074266300


   @aslakhellesoy as the very first message in this PR explains already, the capability for Surefire to order test methods is already implemented -- albeit implemented in a way that Tibor would prefer that we change to merge the feature into Surefire.
   
   Mainly we just need to follow the steps Tibor outlined so that Surefire can order not just classes but also methods. Happy to comment on the work on test it out but won't have time to implement it myself for the next few weeks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1070772326


   How are you @winglam?
   We have discussed your pull request in #169 and it seems that @aslakhellesoy would pay an attention.
   @winglam do you have time to participate?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1073870512


   @winglam 
   @aslakhellesoy 
   See my comment https://github.com/apache/maven-surefire/pull/348#issuecomment-1022724116
   and see whatever surefire provider implementation and try to find `run order` or `runOrder`. Not the `TestListResolver` as many people think. The `AbstractSurefireMojo` has to prepare the string upon `-Dtest=...` and serialize it for later use in RunOrder. Then it should be possible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077771434


   @aslakhellesoy Thanks for the clarification. I was aware from the https://issues.apache.org/jira/browse/SUREFIRE-2041 issue that you wanted a global test method ordering feature that supports the interleaving of test methods across test classes and I agree that it may not be realistic to attempt. 
   
   More importantly, I'm not sure how one would implement such a feature. Namely, how should one handle class setup and teardown methods (e.g., @BeforeClass from JUnit) for an order such as testing.Y#a,testing.X#c,testing.Y#b? Should one run _testing.Y#beforeClass_,testing.Y#a,_testing.X#beforeClass_,testing.X#c,_testing.Y#beforeClass_,testing.Y#b? What if running testing.Y#beforeClass twice without running testing.Y#afterClass causes a failure? We could then run _testing.Y#afterClass_ after testing.Y#a and before _testing.X#beforeClass_ but then that can add substantial time cost to running tests and still cause unexpected failures if these class setup and teardown methods are not expected to run twice in one JVM.
   
   Not supporting a global test method ordering feature is a purposeful design feature when I did my implementation of test method ordering. For the same reason we may not want to support the feature of interleaving test classes across different modules (e.g., module1.testing.Y,module2.testing.X,module1.testing.Z), I'm not sure whether a global test method ordering feature should be supported or not based on the way modules and test classes are defined and commonly used. I believe there is still much value to support test method ordering within classes (and also the ordering of test classes) and the changes I have in this PR accomplishes both test class ordering and test method ordering, just not the interleaving of test methods across classes and in the way that Tibor suggests us to do so for this feature.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] aslakhellesoy commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
aslakhellesoy commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1070779077


   I'll try to help move this PR forward. I'll start by trying to bring this branch uptodate with master. What's the easiest way to collaborate? I could send a pr to the `TestResearchIllinois` repo against the existing `umaster-tms` branch perhaps?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1070857800


   @aslakhellesoy
   The code is not suited to rebase because this code has some issues. That's the reason why I wanted to save the spare time of @winglam and I wrote 5 steps on how to implement the feature right way and fast enough, see the comments written since of January 27 2022.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] aslakhellesoy commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
aslakhellesoy commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1075176203


   > the capability for Surefire to order test methods is already implemented
   
   @winglam your implementation of test method ordering can order tests within the same class, but it cannot order them globally. 
   
   For example, if you specify `-Dtest=testing.Y#a,testing.X#c,testing.Y#b`, then the execution order will not be what you specified. It will be:
   
   * `testing.Y#a`
   * `testing.Y#b`
   * `testing.X#c`
   
   This is because Surefire runs one test class at a time. Supporting a global ordering of test methods would require such a significant refactoring that I'm not sure it's realistic to attempt it. The JUnit 5 team has said they won't support it: https://github.com/junit-team/junit5/discussions/2857
   
   As I have described in [SUREFIRE-2041](https://issues.apache.org/jira/browse/SUREFIRE-2041) I think the best we can do is an explicit class ordering.
   
   I have implemented class ordering in https://github.com/apache/maven-surefire/pull/495, inspired from this PR but with a different implementation. I'm waiting for @Tibor17 to approve the running of the GitHub Action in that PR, and also to review the code.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077771434


   @aslakhellesoy Thanks for the clarification. I was aware from the https://issues.apache.org/jira/browse/SUREFIRE-2041 issue that you wanted a global test method ordering feature that supports the interleaving of test methods across test classes and I agree that it may not be realistic to attempt. 
   
   More importantly, I'm not sure how one would implement such a feature. Namely, how should one handle class setup and teardown methods (e.g., @BeforeClass from JUnit) for an order such as testing.Y#a,testing.X#c,testing.Y#b? Should one run _testing.Y#beforeClass_,testing.Y#a,_testing.X#beforeClass_,testing.X#c,_testing.Y#beforeClass_,testing.Y#b? What if running testing.Y#beforeClass twice without running testing.Y#afterClass causes a failure? We could then run _testing.Y#afterClass_ after testing.Y#a and before _testing.X#beforeClass_ but then that can add substantial time cost to running tests and still cause unexpected failures if these class setup and teardown methods are not expected to run twice in one JVM.
   
   Not supporting a global test method ordering feature is a purposeful design feature when I did my implementation of test method ordering. For the same reason we may not want to support the feature of interleaving test classes across different modules (e.g., module1.testing.Y,module2.testing.X,module1.testing.Z), I'm not sure whether a global test method ordering feature should be supported or not based on the way modules and test classes are defined and commonly used. I believe there is still much value to support test method ordering within classes (and also the ordering of test classes). The feature I have in this PR accomplishes both test class ordering and test method ordering, just not the interleaving of test methods across classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] aslakhellesoy edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
aslakhellesoy edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1073805631


   Hi @winglam - I have created a JIRA issue related to this: https://issues.apache.org/jira/browse/SUREFIRE-2041, with a related PR: https://github.com/apache/maven-surefire/pull/495
   
   In that JIRA issue I have outlined why I don't think ordering test methods is possible (only classes). I'd be interested in your thoughts about this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022720153


   The `TestListResolver` does not do the trick. It is only a filter. You should not use it.
   The whole trick is done in every Surefire Provider implementation. So in this case, you have to modify the providers.
   I think this snap code explains how we do it, see the `JUnit4Provider`:
   ```
       private TestsToRun scanClassPath()
       {
           final TestsToRun scannedClasses = scanResult.applyFilter( jUnit4TestChecker, testClassLoader );
           return runOrderCalculator.orderTestClasses( scannedClasses );
       }
   ```
   The problem is that we do not work with methods. Only classes.
   And we should remember JUnit5 because it know scripts, dynamic tests, etc.
   
   So all you have to do is to serialize run order (enum + data), see `BooterSerializer` and deserialize it via `BooterDeserializer`.
   It means that you have transfered the config for `RunOrderCalculator` fom plugin JVM to the forked JVM. Similar serialization trick in the in-plugin JVM.
   Then you should obtain the instance of `RunOrderCalculator` in the particular provider. The next step is to modify the section I mentioned above which would include the methods as well.
   
   The system property and TestListResolver parts can be avoided.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022724116


   It would help you and us if you follow these steps, in separate commit, and each step would have unit tests:
   1. serialize run order config + test
   2. deserialize config + test
   3. create deserialized instance of run order calculator, verify the config, and write a test
   4. adjust the provider, isolate it in a test
   5. run an integration test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022728134


   Of course the documentation should say that `forkCount > 1` does not guarantee the order of classes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1034303837


   @Tibor17 Thanks a lot for the detailed information, especially the list of steps. 
   
   I still have these changes on my list of todos and hope to get to them soon. Hopefully by the time I have something ready PR 440 would be merged by then. I'll reach out again once I have made some progress on some of the steps.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] winglam commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1072429832


   @Tibor17 and @aslakhellesoy I would be happy to participate in the discussion for this feature. Unfortunately, I would not have time to implement the five-step process that Tibor laid out by myself until this May. 
   
   @aslakhellesoy If you are able to implement the five steps before then, please feel free to do so. I would be happy to look over a PR and work with you to get this feature out the door. Please let me know if this option interests you and how may I help.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] aslakhellesoy commented on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
aslakhellesoy commented on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1077859361


   @winglam are you by any chance working on a Test Case Prioritization engine? I am :-) I went on a little [twitter rant](https://twitter.com/aslak_hellesoy/status/1506225693792903171) about it yesterday.
   
   *If* JUnit implemented global test method ordering, I think `@BeforeClass` and `@AfterClass` should still only run once. before the first test and after the last one, even if interleaved with other classes. Maybe a new JUnit `TestEngine` could implement this.
   
   It's still possible to do TCP with class ordering. The APFD would be lower than with a method ordering, but still better than no ordering!
   
   FWIW, Cucumber-Ruby and Cucumber-JS *do* support global scenario ordering - they can run scenarios in any order even if they are in different files.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-surefire] Tibor17 edited a comment on pull request #348: Enabling Surefire to run test classes and test methods in any order specified by a new runOrder

Posted by GitBox <gi...@apache.org>.
Tibor17 edited a comment on pull request #348:
URL: https://github.com/apache/maven-surefire/pull/348#issuecomment-1022724116


   It would help you and us if you have followed these steps, in separate commits, and each step would have unit tests:
   1. serialize run order config + test
   2. deserialize config + test
   3. create deserialized instance of run order calculator, verify the config, and write a test
   4. adjust the provider, isolate it in a test
   5. write an integration test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org