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 2020/07/31 21:45:47 UTC
[GitHub] [maven-surefire] winglam opened a new pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
winglam opened a new pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309
The changes in this pull request addresses issue SUREFIRE-756. Namely, the changes include
- Output of the random seed used to generate a particular random test order when -Dsurefire.runOrder=random is set
- Ability to replay a previously observed random test order by setting -Dsurefire.seed and -Dfailsafe.seed to the seed that observed the random test order
- Tests to ensure that the setting of the _same_ random seeds do create the _same_ test orders and _different_ random seeds do create _different_ test orders. Note that the inherent randomness of the orders does mean the tests can be flaky (nondeterministically pass or fail without changes to the code). The current tests have a rate of 0.4% ```(1/3)^5``` of failing. Increasing the number of tests (3) or the number of times to loop (5) would decrease the odds of the test failing.
----------------------------------------------------------------
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] rmannibucau commented on a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464988133
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
@Tibor17 fact it creates less parameters is irrelevant - in particular today we have a tons of parameters. Fact it makes it harder to use, integrate and is more error prone for end users is what counts at the end and here having a dedicated variable for the replay is highly relevant IMHO. Also note that having the seed is not always enough to replay a test suite (in particular in random mode) so you can just need to capture the test order and replay it explicitly (a runOrder=fromFile and runOrderFile=/path would be more relevant there).
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464657488
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerStartupConfigurationTest.java
##########
@@ -197,7 +197,7 @@ private ProviderConfiguration getProviderConfiguration()
new TestRequest( Arrays.asList( getSuiteXmlFileStrings() ), getTestSourceDirectory(),
new TestListResolver( "aUserRequestedTest#aUserRequestedTestMethod" ) );
- RunOrderParameters runOrderParameters = new RunOrderParameters( RunOrder.DEFAULT, null );
+ RunOrderParameters runOrderParameters = new RunOrderParameters( RunOrder.DEFAULT, null, null );
Review comment:
detto here
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464655894
##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -3051,6 +3056,17 @@ private void warnIfIllegalTempDir() throws MojoFailureException
}
}
+ private void printDefaultSeedIfNecessary()
Review comment:
These are the test classes, `AbstractSurefireMojoJava7PlusTest` and `AbstractSurefireMojoTest`, where we add the tests for `AbstractSurefireMojo`.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464650686
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
> Do let me know if that's what is desired here.
Yes this is what I mean. Basically, it is easy to parse the number. You can use regex or a custom string spliterator.
----------------------------------------------------------------
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] winglam commented on a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
winglam commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464651066
##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -3051,6 +3056,17 @@ private void warnIfIllegalTempDir() throws MojoFailureException
}
}
+ private void printDefaultSeedIfNecessary()
Review comment:
I can certainly add a test or two that covers this method. If there are other methods that I should cover or if there are particular tests that I should extend, do let me know.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464645318
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
Did you take over the changes from an old PR?
We spoke about this parameter that we already have the config parameter named `runOrder`. And so that it should contain the seed too, let's suppose `<runOrder>random1234567890</runOrder>`.
The reason is that we have too many config parameters and this one is not urgent due to we are able to specify the seed in one business parameter `runOrder`.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464646617
##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -3051,6 +3056,17 @@ private void warnIfIllegalTempDir() throws MojoFailureException
}
}
+ private void printDefaultSeedIfNecessary()
Review comment:
This abstract MOJO class is used in the Mockito tests. Can you cover all your changes and extend the existing tests? It would be cool to have tests maybe for larger methods too.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464657345
##########
File path: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/BooterDeserializerProviderConfigurationTest.java
##########
@@ -276,7 +276,7 @@ private ProviderConfiguration getTestProviderConfiguration( DirectoryScannerPara
new TestRequest( getSuiteXmlFileStrings(), getTestSourceDirectory(),
new TestListResolver( USER_REQUESTED_TEST + "#aUserRequestedTestMethod" ),
RERUN_FAILING_TEST_COUNT );
- RunOrderParameters runOrderParameters = new RunOrderParameters( RunOrder.DEFAULT, null );
+ RunOrderParameters runOrderParameters = new RunOrderParameters( RunOrder.DEFAULT, null, null );
Review comment:
It would be worth to test the non-null value.
----------------------------------------------------------------
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 merged pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309
----------------------------------------------------------------
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] winglam commented on a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
winglam commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r465161332
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
> capture the test order and replay it explicitly (a runOrder=fromFile and runOrderFile=/path would be more relevant there)
I actually have some changes (in a separate local branch) that would accomplish this as well and plan to submit it after this PR is merged. I could still use some help/advice on how to capture the test order (with or without running tests). The goal of my changes would be to empower runOrder to be more customizable and deterministic with seeds, passing in a file, controlling not just test class order but also test method order, etc.
I have the changes for merging `failsafe.seed` and `surefire.seed` with the `runOrder` parameter ready, but will leave those unpushed for now as I am fine with merging them or keep them separate (as they are now or moving it, etc.).
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
> capture the test order and replay it explicitly (a runOrder=fromFile and runOrderFile=/path would be more relevant there)
I actually have some changes (in a separate local branch) that would accomplish this as well and plan to submit it after this PR is merged. I could still use some help/advice on how to capture the test order (with or without running tests). The goal of my changes would be to empower runOrder to be more customizable and deterministic with seeds, passing in a file, controlling not just test class order but also test method order, etc.
I have the changes for merging `failsafe.seed` and `surefire.seed` with the `runOrder` parameter ready, but will leave those unpushed for now as I am fine with merging them or keep them separate (as they are now or moving it, etc.).
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464949748
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
@rmannibucau This was we would have too many config params. Having it in one means that it is about the same business. Here you are setting only run order and so that you say "random" is the same business value with the "seed".
Alternatively it can be done another way with the extensions where this parameter may become an interface. It means that the parameter "runOrder" would become an element what is called complexType in the XML and it may have specific nested elements for the concrete implementation of the interface, and the "seed" would be only for the "random".
----------------------------------------------------------------
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] rmannibucau commented on a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464810483
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
-1, it is two separate params and must be overridable with a system prop easily and *independently* so it is good like that.
You can move it to other parameters or even a map of runOrderParameters to avoid more params but having to merge the steing is bad for end users and not that consistent with the config IMHO.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464650686
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
> Do let me know if that's what is desired here.
Yes this is what I mean. Basically, it is easy to parse the number. You can use regex or the string spliterator.
----------------------------------------------------------------
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 #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#issuecomment-694240981
I will have a look in few hours. T
Dňa št 17. 9. 2020, 0:30 winglam <no...@github.com> napísal(a):
> @Tibor17 <https://github.com/Tibor17> just a reminder that since we last
> spoke, I've proofread my changes again, did the refactoring you asked,
> cleaned up some unnecessary changes, and added a test for the
> SurefireReflector. If there's anything else needed, let me know and I'll
> try to get to it soon
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/maven-surefire/pull/309#issuecomment-693699949>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAH7ER3EGY55ND5PWQLF2B3SGE4BTANCNFSM4PQ6K2MA>
> .
>
----------------------------------------------------------------
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] winglam commented on a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
winglam commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464647763
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
These changes were not taken from an old PR, but I would be happy to take a look and compare the changes if you can point me to the PR.
As for not adding failsafe.seed or surefire.seed, I can change it so that it just parses the numbers after random as the seed instead. Do let me know if that's what is desired here.
----------------------------------------------------------------
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 #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#issuecomment-694491456
@winglam
First of all the build must be stable, otherwise adding more and more commits would be a disaster.
The build of master is in progress, see https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-surefire/job/master/
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r476757396
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
@winglam
Pls refactor the name `randomSeed` and the property name `failsafe.seed`, and for surefire too.
----------------------------------------------------------------
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] winglam commented on pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
winglam commented on pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#issuecomment-693699949
@Tibor17 just a reminder that since we last spoke, I've proofread my changes again, did the refactoring you asked, cleaned up some unnecessary changes, and added a test for the SurefireReflector. If there's anything else needed, let me know and I'll try to get to it soon
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464659711
##########
File path: surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireReflector.java
##########
@@ -178,10 +178,11 @@ private Object createRunOrderParameters( RunOrderParameters runOrderParameters )
return null;
}
//Can't use the constructor with the RunOrder parameter. Using it causes some integration tests to fail.
- Class<?>[] arguments = { String.class, File.class };
+ Class<?>[] arguments = { String.class, File.class, Long.class };
Constructor<?> constructor = getConstructor( this.runOrderParameters, arguments );
File runStatisticsFile = runOrderParameters.getRunStatisticsFile();
- return newInstance( constructor, RunOrder.asString( runOrderParameters.getRunOrder() ), runStatisticsFile );
+ return newInstance( constructor, RunOrder.asString( runOrderParameters.getRunOrder() ), runStatisticsFile,
Review comment:
This reflector should be tested in `SurefireReflectorTest`.
----------------------------------------------------------------
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 a change in pull request #309: [SUREFIRE-756] Allow ability to capture executed random runOrder for re-play purposes
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #309:
URL: https://github.com/apache/maven-surefire/pull/309#discussion_r464949748
##########
File path: maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java
##########
@@ -310,6 +310,22 @@
@Parameter( property = "failsafe.runOrder", defaultValue = "filesystem" )
private String runOrder;
+ /**
+ * Sets the random seed that will be used to order the tests if {@code failsafe.runOrder} is set to {@code random}.
+ * <br>
+ * <br>
+ * If no seeds are set and {@code failsafe.runOrder} is set to {@code random}, then the seed used will be
+ * outputted (search for "To reproduce ordering use flag -Dfailsafe.seed").
+ * <br>
+ * <br>
+ * To deterministically reproduce any random test order that was run before, simply set the seed to
+ * be the same value.
+ *
+ * @since 3.0.0-M6
+ */
+ @Parameter( property = "failsafe.seed" )
+ private Long randomSeed;
Review comment:
@rmannibucau This way we would have too many config params. Having it in one means that it is about the same business. Here you are setting only run order and so that you say "random" is the same business value with the "seed".
Alternatively it can be done another way with the extensions where this parameter may become an interface. It means that the parameter "runOrder" would become an element what is called complexType in the XML and it may have specific nested elements for the concrete implementation of the interface, and the "seed" would be only for the "random".
----------------------------------------------------------------
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