If you specify a --shuffle for the CLI the tests will be run in a specific order as set by the seed. If no seed is given a default seed is acquired. To be able to run the same order of tests again, this default seed has to be logged. IMO it would be good to do that whenever the --shuffle option is specified.

The seed is already logged here: https://hg.mozilla.org/mozilla-central/file/eab4a81e4457/testing/marionette/client/marionette/runner/base.py#l848 as implemented by bug 984208.

I see. The problem here was that my tests were aborted in the middle of a testrun. So I haven't seen the logging of the seed at the very end. Given that we want to re-use the seed, we should log it before the tests are run.

Hi, I'd like to take a shot at this

You are welcome to work on it. In case of questions please let us know. Otherwise have a look at the provided link in how to get started.

Hi, I'm new to the community. Can I work on this?

Yes, you are welcome to work on this bug. Please check the user story above in how to get started. In case of questions please ask here or on irc.mozilla.org in the #ateam channel.

I tried to find the file mentioned https://hg.mozilla.org/mozilla-central/file/eab4a81e4457/testing/marionette/client/marionette/runner/base.py#l848 but it seems that the directory client/marionette no longer exists. Can you please help me in locating the file?

You can use SearchFox to search through all of central, and searching for "seed" in testing/marionette seems to give a few good pointers to where you need to make your change: https://searchfox.org/mozilla-central/search?q=seed&case=false&regexp=false&path=testing%2Fmarionette

(In reply to Andreas Tolfsen ‹:ato› from comment #8) > You can use SearchFox to search through all of central, and searching > for "seed" in testing/marionette seems to give a few good pointers > to where you need to make your change: > > https://searchfox.org/mozilla-central/ > search?q=seed&case=false&regexp=false&path=testing%2Fmarionette Thank you for helping. I'll make the change.

namrata, thank you for uploading the patch. If you think it is ready to be reviewed please make sure to open the patch in mozreview and set the reviewer flag to me. Thanks.

Comment on attachment 8959503 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/228302/#review234994 Please use `hg histedit` to squash this extra commit into the first one. Note that usually you want to use the `--amend` option of `hg commit` to combine directly after editing.

Comment on attachment 8959502 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/228292/#review234996

Comment on attachment 8959503 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/228302/#review234994 Sorry, I'm new. I'll fix this right away.

Comment on attachment 8959502 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/228292/#review234996 I have squashed the commits. Should I set you as the reviewer again?

Comment on attachment 8959502 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/228292/#review235732 ::: commit-message-80b47:9 (Diff revision 2) > +*** > +Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. > + > +MozReview-Commit-ID: E9ISYqNEMZA > + > +Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. Looks like the history rewrite didn't succeed here given that you have the same line printed multiple times. So use `hg histedit` again, and choose `m` (modify) to fix the commit message as following: 1) Use a short summary of what this change is actually modifying 2) Use a more detailed description separated by an empty line for more information. 3) Only have the summary and details listed once. ::: testing/marionette/harness/marionette_harness/runner/base.py:929 (Diff revision 2) > - for run_tests in self.mixin_run_tests: > - run_tests(tests) > if self.shuffle: > self.logger.info("Using shuffle seed: %d" % self.shuffle_seed) > + for run_tests in self.mixin_run_tests: > + run_tests(tests) When you run the Marionette tests via `./mach marionette test --shuffle` you will notice that the shuffle seed will still be printed at the end of the test run. Please note that the for loop here doesn't actually run the main chunk of tests. This is done by the loop above. So as best move the two lines up to after `self.logger.suite_start()`.

Hi namrata, thank you for the patch! I checked it and added some details to the review request, which you want to get fixed. Let me know if you have troubles with histedit, and we can solve it on IRC.

Sadly we haven't heard from namratamukhija for about 2 months. So I'm resetting the assignee to allow others to work on it.

Comment on attachment 8974126 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/242420/#review248338 Code analysis found 1 defect in this patch: - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: testing/marionette/harness/marionette_harness/runner/base.py:890 (Diff revision 1) > self.marionette.delete_session() > > tests_by_group = defaultdict(list) > for test in self.tests: > tests_by_group[test['group']].append(test['filepath']) > + Error: Blank line contains whitespace [flake8: W293]

Hi, I fixed those two issues in namrata's patch. Wanted to credit namratamukhija's effort but I didn't know how.

(In reply to Alvaro Reina (:AlvaroRe) from comment #24) > Hi, I fixed those two issues in namrata's patch. Wanted to credit > namratamukhija's effort but I didn't know how. Given that this is just a move of a single code block, I don't see that we have to add any credits. I will have a look at your patch in a moment.

Comment on attachment 8974126 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/242420/#review248470 ::: commit-message-80b47:1 (Diff revision 2) > +Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. The first line of the commit message should only contain a short overview of what has been changed. The component is optional but nice to have. So just say: "Bug 1141118 - [marionette] In shuffle mode log the seed value before the tests are run." ::: testing/marionette/harness/marionette_harness/runner/base.py:892 (Diff revision 2) > tests_by_group = defaultdict(list) > for test in self.tests: > tests_by_group[test['group']].append(test['filepath']) > > + if self.shuffle: > + self.logger.info("Using shuffle seed: %d" % self.shuffle_seed) In comment 18 of the bug I mentioned to move it after the `suite_start()` call. So that it would look like: > SUITE-START | Running 1 tests > Using shuffle seed: 1296588404415960593

Comment on attachment 8974126 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. https://reviewboard.mozilla.org/r/242420/#review248338 Also please fix this linter failure.

That should be it, no more confusion between before and after. There's a yellow line of code between revision 2 and 3 , i don't know what it means. I setted up a virtual machine for everything mozilla related after revision 2 (to get rid of Windows CRLF mainly), and I think that yellow line warns of a difference of versions, so I don't know if I screwed up again.

Comment on attachment 8959502 [details] Bug 1141118 - Fixed logging of seed before the tests are run. The seed is now logged before the tests are run. This enables re-using of the seed, if required. You submitted once more a new review request. I would say that before you push a patch on a different bug in the future lets have a chat on IRC so we can fix that problem. Otherwise this patch looks fine now. Thank you for the update!

Comment on attachment 8975188 [details] Bug 1141118 - [marionette] In shuffle mode log the seed value before the tests are run. https://reviewboard.mozilla.org/r/243530/#review249518

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/02f1065a7bea [marionette] In shuffle mode log the seed value before the tests are run. r=whimboo