Closed Bug 1435552 Opened 2 years ago Closed Last year

Remove NO_EM_RESTART from test harnesses

Categories

(Testing :: General, enhancement)

Version 3
enhancement
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: gbrown, Assigned: manishkk)

Details

Attachments

(1 file, 1 obsolete file)

Various harness/runner/test automation support files set the environment variable NO_EM_RESTART, but it looks like this is unused in Firefox.
need to remove NO_EM_RESTART from mozharness_test.py file only or from everywhere?
https://searchfox.org/mozilla-central/search?q=NO_EM_RESTART&path=
Flags: needinfo?(gbrown)
I think NO_EM_RESTART should be removed from everywhere.

It looks to me like support for using NO_EM_RESTART was removed way back in https://hg.mozilla.org/mozilla-central/rev/8c7df4612f8e. Since then various test harnesses and test runners have been setting it or ensuring that it is not set, but as far as I can tell, this environment variable has no effect on Firefox.

Do you want to take this? Otherwise, I can clean this up this week.
Flags: needinfo?(gbrown)
Assignee: nobody → 1991manish.kumar
This is my first testing bug, I am taking this.
After removing the NO_EM_RESTART, How can I test the changes?
Flags: needinfo?(gbrown)
Thanks Manish.

To test quickly, I suggest first running some 'mach test' commands locally. 

If that doesn't obviously break, run a few test jobs on try: A variety of test suites -- perhaps a talos, a mochitest, a reftest task -- on a variety of platforms, including at least Android and osx. To set this up I would use 'mach try fuzzy'.
Flags: needinfo?(gbrown)
Attached patch Patch_Bug1435552 (obsolete) — Splinter Review
Please review!
Attachment #9005476 - Flags: review?(gbrown)
Comment on attachment 9005476 [details] [diff] [review]
Patch_Bug1435552

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

This looks great, except for the minor lint problem.

Also, did you run some tests on try? I'd like to see a link to that.

::: testing/mozbase/mozrunner/mozrunner/base/device.py
@@ +29,4 @@
>             'MOZ_LOG': 'signaling:3,mtransport:4,DataChannel:4,jsep:4,MediaPipelineFactory:4',
>             'R_LOG_LEVEL': '6',
>             'R_LOG_DESTINATION': 'stderr',
> +           'R_LOG_VERBOSE': '1',}

'mach lint' reports a problem here:

/home/gbrown/src/testing/mozbase/mozrunner/mozrunner/base/device.py
  32:32  error  missing whitespace after ','  E231 (flake8)

You can probably just add a space, like:

   'R_LOG_VERBOSE': '1', }
Attachment #9005476 - Flags: review?(gbrown) → review-
I tried './mach test' which is not working!

Please check updated patch!
Attachment #9005476 - Attachment is obsolete: true
Attachment #9005613 - Flags: review?(gbrown)
Comment on attachment 9005613 [details] [diff] [review]
Patch2_Bug1435552

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

I ran 'mach test testing/mochitest/tests/Harness_sanity' and similar commands -- all fine.

Here's a try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2879b53cf24c682925395b7196c5dfad173b178

This looks great. Thanks!


Do you need help with check-in?
Attachment #9005613 - Flags: review?(gbrown) → review+
yes
> 
> Do you need help with check-in?
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c47e4fa9dba8
Remove NO_EM_RESTART from test harnesses; r=gbrown
https://hg.mozilla.org/mozilla-central/rev/c47e4fa9dba8
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.