Closed Bug 1435552 Opened 2 years ago Closed Last year

Remove NO_EM_RESTART from test harnesses


(Testing :: General, enhancement)

Version 3
Not set


(firefox63 fixed)

Tracking Status
firefox63 --- fixed


(Reporter: gbrown, Assigned: manishkk)



(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 file only or from everywhere?
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 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]

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/
@@ +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:

  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]

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:

This looks great. Thanks!

Do you need help with check-in?
Attachment #9005613 - Flags: review?(gbrown) → review+
> Do you need help with check-in?
Pushed by
Remove NO_EM_RESTART from test harnesses; r=gbrown
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.