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

RESOLVED FIXED in Firefox 48

Status

Testing
Mochitest
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Away for a while, Unassigned)

Tracking

unspecified
mozilla48
Points:
---

Firefox Tracking Flags

(e10s+, firefox48 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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?
(Reporter)

Comment 1

2 years ago
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
(Reporter)

Comment 3

2 years ago
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?
(Reporter)

Comment 5

2 years ago
Yes, thanks!
(Reporter)

Comment 6

2 years ago
Created attachment 8731872 [details] [diff] [review]
Indicate whether tests ran in e10s mode in the mochitest-* summary
Attachment #8731872 - Flags: review?(mconley)
(Reporter)

Updated

2 years ago
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+
(Reporter)

Comment 8

2 years ago
(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. :)
(Reporter)

Comment 10

2 years ago
Already did: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cbd6acab11b

Try seems to be happy!

Comment 11

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/eafefe59f701

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eafefe59f701
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.