Closed Bug 1256984 Opened 4 years ago Closed 4 years ago

Add a desktop notification notifying whether you're running tests in e10s or non-e10s mode

Categories

(Testing :: Mochitest, defect, P3)

defect

Tracking

(e10s+, firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox48 --- fixed

People

(Reporter: ehsan, Unassigned)

Details

Attachments

(1 file)

I keep running into this mistake where I run for example mochitests without --e10s trying to test something in e10s mode and nothing on the screen makes it clear that the browser is in non-e10s mode, so I think the test passes when in reality it may not.

I was thinking of adding a desktop notification that pops up when you run a test saying whether you're in e10s mode, similar to what ./mach build does when a long running build finishes.  If we do it in mach, we can reuse that infrastructure <https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mach_commands.py#485>

Mike, what do you think about this?  Nice, or too crazy?
Actually needinfoing Mike!
Flags: needinfo?(mconley)
I think making it clear to the tester what mode was tested is a good idea. I'm not 100% certain the long-build notification is the right choice.

Perhaps we could pump out the state of what the browser was in when it was tested right at the end near the pass / fail / todo readout? That could also be useful when test logs are pasted back and forth.
Flags: needinfo?(mconley)
tracking-e10s: --- → +
Priority: -- → P3
Makes sense.

In the chrome process, we can check to see whether we're in e10s mode using gMultiProcessBrowser.  Is there something similar in the content process that we can check?  Anything accessible from JS would do!

Thanks.
Flags: needinfo?(mconley)
Do you need SpecialPowers.isMainProcess(), perhaps?
Yes, thanks!
Flags: needinfo?(mconley)
Comment on attachment 8731872 [details] [diff] [review]
Indicate whether tests ran in e10s mode in the mochitest-* summary

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

::: testing/mochitest/runtests.py
@@ +2174,5 @@
>  
>              if result == -1:
>                  break
>  
> +        e10s_mode = "e10s" if options.e10s else "non-e10s"

This is going to stop being true when bug 1243083 lands. We should probably alert the folks in that bug that this will need to be changed once the default shifts.

::: testing/mochitest/tests/SimpleTest/TestRunner.js
@@ +448,5 @@
>          }
>  
>          SpecialPowers.unregisterProcessCrashObservers();
>  
> +        var e10sMode = SpecialPowers.isMainProcess() ? 'non-e10s' : 'e10s';

let, not var. Also, let's use double-quotes for consistency.
Attachment #8731872 - Flags: review?(mconley) → review+
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #7)
> Comment on attachment 8731872 [details] [diff] [review]
> Indicate whether tests ran in e10s mode in the mochitest-* summary
> 
> Review of attachment 8731872 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mochitest/runtests.py
> @@ +2174,5 @@
> >  
> >              if result == -1:
> >                  break
> >  
> > +        e10s_mode = "e10s" if options.e10s else "non-e10s"
> 
> This is going to stop being true when bug 1243083 lands. We should probably
> alert the folks in that bug that this will need to be changed once the
> default shifts.

Will do, thanks!
Also, it might be worth pushing this to try. I doubt that any of our automation attempts to parse those last few strings, but you never know - we might have just pulled the rug out from under some Python script that expects there to not be that "Mode" line.

Up to you whether you want to take the gamble. :)
Already did: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cbd6acab11b

Try seems to be happy!
https://hg.mozilla.org/mozilla-central/rev/eafefe59f701
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.