984208 - [Marionette Client] Modifying shuffle to put its seed outside

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Walter Chen[:ypwalter][:wachen]]

As Szu-Yu Chen [:aknow] recommends in Bug 912890, random seed explicitly before calling shuffle(), in this way, we could keep the seed (print it out) and repeat the same sequence if we need.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

We'll need to update the runner to take in a seed, and if none is passed, use a new one and print it out. To pass in a seed, we can add a --shuffle-seed option near here (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#461) oo take in the seed value. 

We'll need to update https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#945 to take in this seed value if given, or use a new seed value and print it out.

Comment 2

[Kevin Chang]

Hi. I would like to take this up as my first bug.

Comment 3

[Walter Chen[:ypwalter][:wachen]]

not a critical blocker to MTBF, but it is a good-to-have

Comment 4

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Kevin Chang from comment #2)
> Hi. I would like to take this up as my first bug.

Sorry I missed this message, hopefully you're still around! The instructions to get this working are as follows:

You'll need a local copy of mozilla-central to make changes and test them:

https://hg.mozilla.org/mozilla-central/

the Marionette client lives in 
<mozilla-central>/source/testing/marionette/client/marionette/

To start running tests, go into the Marionette client directory and create a virtual environment:

virtualenv my_venv

(if you do not have virtualenv, install it using `pip install virtualenv`)

Then activate it and install marionette client:

. my_venv/bin/activate
python setup.py develop

To run a test suite, you can do:

python runtests.py --binary=<path to your firefox executable> --shuffle --type=browser tests/unit/unit-test.ini

This should start up a browser and run tests against it. The firefox executable path is an executable file within your Firefox Application named 'firefox' on Mac/Linux and 'firefox.exe' in Windows (not sure about Marionette's compatibility with Windows though!)

To address this particular issue, we want to print out the seed at the end of the test run. We also want to be able to pass in a seed that will be used to set the randomness. To do this, we'll need to update the runner to take in a seed, and if none is passed, use a new one and print it out in both cases. To pass in a seed, we can add a --shuffle-seed option near here (https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#461) to take in the seed value. 

We'll need to update https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#945 to take in this seed value if given, or use a new seed value, and print it out. To print it out, you can add a print line after the for loop here https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#822

Comment 5

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Malini Das [:mdas] from comment #4)
There's a typo:
> 
> To run a test suite, you can do:
> 
> python runtests.py --binary=<path to your firefox executable> --shuffle
> --type=browser tests/unit/unit-test.ini

should be:

python runtests.py --binary=<path to your firefox executable> --shuffle --type=browser tests/unit/unit-tests.ini

Comment 6

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Malini Das [:mdas] from comment #4)
another update. I need to use changeset based links
> We'll need to update
> https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/
> marionette/runner/base.py#945 to take in this seed value if given, or use a
> new seed value, and print it out. To print it out, you can add a print line
> after the for loop here
> https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/
> marionette/runner/base.py#822

I meant: https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py?rev=60d427dbb7dc#986 for the first link

and: 
https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py?rev=60d427dbb7dc#836

for the last one. Instead of a print line, do self.logger.info(<seed info>)

Comment 7

[Vikas Mishra [:mishravikas]]

Attachment: fix_984208.patch

Comment 8

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8408307 [details] [diff] [review]
fix_984208.patch

Review of attachment 8408307 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome, this works just as expected!

I forgot to mention, I have one improvement to add before I r+ it. Can you print out the seed only if --shuffle is passed? Thanks!

::: testing/marionette/client/marionette/runner/base.py
@@ +830,5 @@
>          self.marionette.cleanup()
>  
>          for run_tests in self.mixin_run_tests:
>              run_tests(tests)
> +        self.logger.info("Using seed where seed is:%d" % self.shuffle_seed)

Can you print this only if --shuffle has been passed?

Comment 9

[Malini Das [:mdas] - Away, not checking bugmail]

Vikas, are you around to update the patch?

Comment 10

[Luv Agarwal(:lagarwal)]

Hi, I am interested in completing the remaining part of this bug.

Comment 11

[Luv Agarwal(:lagarwal)]

Attachment: fix_984208.patch

added option to input the seed for shuffle

Comment 12

[Malini Das [:mdas] - Away, not checking bugmail]

Can you unbitrot the patch? It doesn't apply cleanly to the updated repository and we'll need to have it apply in order to test and land, thanks!

Comment 13

[Malini Das [:mdas] - Away, not checking bugmail]

oh, in case you are unfamiliar with the word 'unbitrot', it means "update the patch since things have changed" :)

Comment 14

[Luv Agarwal(:lagarwal)]

Attachment: fix_984208.patch

added option to input the seed for random.shuffle

Comment 15

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8434257 [details] [diff] [review]
fix_984208.patch

Review of attachment 8434257 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch! I'll remove the extra whitespace this time and push the patch.

::: testing/marionette/client/marionette/runner/base.py
@@ +474,5 @@
> +        self.add_option('--shuffle-seed',
> +                        dest='shuffle_seed',
> +                        type=int,
> +                        default=random.randint(0, sys.maxint),
> +                        help='Use given seed to shuffle tests')                

Next time, remove the extra whitespace at the end of the line

Comment 16

[Malini Das [:mdas] - Away, not checking bugmail]

i'll land this when the tree opens

Comment 17

[Malini Das [:mdas] - Away, not checking bugmail]

landed on m-i

 https://hg.mozilla.org/integration/mozilla-inbound/rev/9347f1e2551a

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9347f1e2551a