[Marionette Client] Modifying shuffle to put its seed outside

RESOLVED FIXED in mozilla32

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: wachen, Assigned: lagarwal)

Tracking

({pi-marionette-runner})

unspecified
mozilla32
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=py][mentor=mdas])

Attachments

(3 attachments)

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.
Whiteboard: [good first bug][lang=py][mentor=mdas]
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

5 years ago
Hi. I would like to take this up as my first bug.
not a critical blocker to MTBF, but it is a good-to-have
Blocks: MTBF-meta
(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
Whiteboard: [good first bug][lang=py][mentor=mdas] → [good first bug][lang=py][mentor=mdas][runner]
(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
(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>)
Whiteboard: [good first bug][lang=py][mentor=mdas][runner] → [good first bug][lang=py][mentor=mdas]
Attachment #8408307 - Flags: review?(mdas)
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?
Attachment #8408307 - Flags: review?(mdas)
Vikas, are you around to update the patch?
Assignee: nobody → vikasmishra95
Assignee

Comment 10

5 years ago
Hi, I am interested in completing the remaining part of this bug.
Assignee

Comment 11

5 years ago
added option to input the seed for shuffle
Attachment #8434005 - Flags: review?(mdas)
Assignee

Updated

5 years ago
Flags: needinfo?(mdas)
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!
Assignee: vikasmishra95 → luvagarwal1995su
Flags: needinfo?(mdas)
oh, in case you are unfamiliar with the word 'unbitrot', it means "update the patch since things have changed" :)
Assignee

Comment 14

5 years ago
added option to input the seed for random.shuffle
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
Attachment #8434257 - Flags: review+
i'll land this when the tree opens
https://hg.mozilla.org/mozilla-central/rev/9347f1e2551a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Reporter

Updated

5 years ago
Blocks: MTBF-Marionette
No longer blocks: MTBF-meta
You need to log in before you can comment on or make changes to this bug.