923720 - Remote discovery pane should not be loaded for restart functional and remote tests

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As what I have seen right now is that our Addons restart tests still don't change the discovery pane url to a local address. That causes timeouts if network issues exist.

We don't want to use remote content for our functional tests, so that we can run them as fast as possible. Lets get this changed.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jonathan, would you have interest in working on it? Looks like a great time safer for us. A quick test has shown me that for each call of the remote discovery pane we can save at least 2s.

Some of the tests already set this pref via addons.setDiscoveryPaneURL() like testAddons_changeTheme.js, but not all.

Comment 2

[Jonathan French (:jfrench)]

Sounds good Henrik, I will have a go.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As long as our tests do not explicitly test the discovery pane all of them should disable the remove version.

Comment 4

[Jonathan French (:jfrench)]

So far so good. I made the change to testAddons_installMultipleExtensions just to start, and then tested it repeatedly pre and post patch. I saw a clock on the wall improvement of about 4-6 seconds for that test (excluding an unrelated failure present).

As far as the changes I'm planning on giving these a discrete block in setupModule()

aModule.addonsManager = new addons.AddonsManager(aModule.controller);
addons.setDiscoveryPaneURL(TEST_DATA);

if that makes sense.. that appears to be the convention in the other tests I've seen using it.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Yeah, such an improvement I have expected. Lets get this done for all the tests which are missing it. Thanks Jonathan!

Comment 6

[Jonathan French (:jfrench)]

So for a reference I checked for tests using setDiscoveryPaneURL() with our new MXR search
http://mxr.mozilla.org/mozmill-tests/search?string=addons.SetDiscoveryPaneURL

Based on that and other investigation, I made changes to functional/restartTests/ (4 tests below), and had a look at functional/testAddons/, remote/restartTests, remote/testAddons, and endurance/. Neither testAddons/ or remote/* appeared to require modifications. I have a question about endurance/ if they are applicable in this work:

endurance/testAddons_OpenAndCloseGetAddonsPane (uses setDiscoveryPaneURL() )
endurance/testAddons_OpenAndCloseExtensionList (looks like it could be a candidate for the change also)

...however the addition just appears to add overhead to that test?.. about 100ms.

Here are the timing improvements I'm seeing with the functional/ tests changed on my (slow) machine:
restartTests/testAddons_installMultipleExtensions (4-6 sec speedup)
restartTests/testAddons_installUninstallHardBlocklistedExtension (perhaps 1-200ms)
restartTests/functiontestAddons_installUninstallSoftBlocklistedExtension (perhaps 1-200ms)
restartTests/testAddons_pluginDisabledAfterRestart (4-5 sec speedup)

Let me know what if any areas of the repo I may have missed.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jonathan French (:jfrench) from comment #6)
> Based on that and other investigation, I made changes to
> functional/restartTests/ (4 tests below), and had a look at
> functional/testAddons/, remote/restartTests, remote/testAddons, and
> endurance/. Neither testAddons/ or remote/* appeared to require
> modifications. I have a question about endurance/ if they are applicable in
> this work:
> 
> endurance/testAddons_OpenAndCloseGetAddonsPane (uses setDiscoveryPaneURL() )
> endurance/testAddons_OpenAndCloseExtensionList (looks like it could be a
> candidate for the change also)

Have you checked any test which makes use of the Add-on manager? Any of those would be hot candidates for this bug.

> ...however the addition just appears to add overhead to that test?.. about
> 100ms.

It shouldn't that much. How often have you run this test in both versions? A mean time would be better to have.

> restartTests/testAddons_installUninstallHardBlocklistedExtension (perhaps
> 1-200ms)
> restartTests/functiontestAddons_installUninstallSoftBlocklistedExtension
> (perhaps 1-200ms)

Do both of them open the add-on manager and are showing the discovery pane? If not they wouldn't need the change.

Comment 8

[Jonathan French (:jfrench)]

(In reply to Henrik Skupin (:whimboo) from comment #7)
> 
> Have you checked any test which makes use of the Add-on manager? Any of
> those would be hot candidates for this bug.

I've reviewed all tests in these locations so far

tests/functional/restartTests/ (14 test folders)
tests/functional/testAddons/ (2 tests)
tests/endurance/ (17 test folders)
tests/remote/restartTests/ (10 test folders)
tests/remote/testAddons/ (1 test)

I also did a MXR search for addonsManager.open() and the results returned are a subset of these folders above.

Is there another search pattern I should use? There are other tests in these folders above, which 'require' addons but they don't open the addon manager or the discovery panel.

> It shouldn't that much. How often have you run this test in both versions? A
> mean time would be better to have.

I rechecked testAddons_OpenAndCloseExtensionList with 30 runs pre/post-change and derived the mean. The modification does improve the run, by about 40ms avg on my (slow) machine. I would guess it may speed up the test on a real node by some smaller amount. This test opens the addon manager but I do not see the discovery panel draw during the run. Let me know what you'd like done for this test.

> > restartTests/testAddons_installUninstallHardBlocklistedExtension (perhaps
> > 1-200ms)
> > restartTests/functiontestAddons_installUninstallSoftBlocklistedExtension
> > (perhaps 1-200ms)
> 
> Do both of them open the add-on manager and are showing the discovery pane?
> If not they wouldn't need the change.

Tests 2 through 4 in each test folder open the addons manager but I don't see the discovery panel draw during the run. Let me know if you'd still like them included.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jonathan French (:jfrench) from comment #8)
> Is there another search pattern I should use? There are other tests in these
> folders above, which 'require' addons but they don't open the addon manager
> or the discovery panel.

Only if addons is required a test can open the add-on manager. We never do this without this library. So it should be safe to say you got all instances.

> I rechecked testAddons_OpenAndCloseExtensionList with 30 runs
> pre/post-change and derived the mean. The modification does improve the run,
> by about 40ms avg on my (slow) machine. I would guess it may speed up the
> test on a real node by some smaller amount. This test opens the addon
> manager but I do not see the discovery panel draw during the run. Let me
> know what you'd like done for this test.

That's correct for this endurance test. Can you imagine why this is happening that we do not open the discovery pane?
 
> > > restartTests/testAddons_installUninstallHardBlocklistedExtension (perhaps
> > > 1-200ms)
> > > restartTests/functiontestAddons_installUninstallSoftBlocklistedExtension
> > > (perhaps 1-200ms)
> > 
> > Do both of them open the add-on manager and are showing the discovery pane?
> > If not they wouldn't need the change.
> 
> Tests 2 through 4 in each test folder open the addons manager but I don't
> see the discovery panel draw during the run. Let me know if you'd still like
> them included.

The discovery pane should be loaded in those two test2.js files. Not sure why you don't see that. Please watch closely again.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jonathan, as we have discovered on bug 835296 it makes more sense to use about:home, and not a local page served via httpd.js. Would you mind to update your local patch accordingly? Thanks.

Comment 11

[Jonathan French (:jfrench)]

Sure, will do.

Comment 12

[Jonathan French (:jfrench)]

Made some progress today. The nature of the about:home change in bug 835296 to me implies there will be many more tests which open the addon manager which can be refactored in this manner. I plan to do that. So far the initial tests re-modified, are showing good performance improvements. I will track the improvements in approx ms for each test, and include those with a first patch when ready.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Interesting. Please let us know which other tests are also affected and can be improved. I'm curious to see how much time we can really save. Looking forward to your patch, Jonathan!

Comment 14

[Jonathan French (:jfrench)]

Will do. There were some tests that open addon manager which already used the original planned change (a local file), but I believe can now use about:home. So that increases the number of tests we can modify, I think. I'll put up a wip patch shortly. The performance numbers vary quite a bit depending on the test, but they are all improving so far. Most of the effort spent is in testing with multiple runs to be sure it's having the desired effect.

I am seeing a range of about -500ms to -5000ms saved depending on the test, so far.

Comment 15

[Jonathan French (:jfrench)]

Attachment: about home discovery pane (default) rev1.1 WIP

WIP patch for default branch, just for visibility to ensure I am on the right track here. Performance improvements listed below on my own, slower machine. I would expect the improvements on real test node to be less.

testAddons_changeTheme/ -500ms
testAddons_enableDisableExtension/ -700ms
testAddons_installMultipleExtensions/ -5000ms *
testAddons_installTheme/ (perhaps -100ms)
testAddons_installUninstallHardBlocklistedExtension/ -5700ms *
testAddons_installUninstallSoftBlocklistedExtension/ -5500ms *
testAddons_pluginDisabledAfterRestart/ -5100ms *

*test occasionally hangs but confirmed in repo also and not related to the patch

Tested via mozmill --restart in:
Windows XP32 SP3
mozmill 2.1-dev latest + patch
latest Nightly 27.0a1 20131016030202 150838

I am continuing on, re-reviewing the other tests.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

You shouldn't test with 2.1-dev. That's an unsupported version. For now it would be very helpful to run those tests via Mozmill 1.5.24 and the mozmill-restart command. You might even want to use ´time mozmill-restart´ to get the overall time for the tests reported.

Comment 17

[Jonathan French (:jfrench)]

Thanks that seems to explain the hangs. Proceeding with 1.5.24.

Comment 18

[Jonathan French (:jfrench)]

So since on windows, time appears to be a reserved keyword rather than a real command (ie. with all the output style choices) I wrote a hacky bash script to do multiple runs for each test, express them in total ms, and average those run times. I'm going to do some runs in 1.5.24 pre and post-patch with the existing modified tests, and then post and update with the performance differences.

Comment 19

[Jonathan French (:jfrench)]

Here's some results. Each restart test was run 10x and averaged. I had started using 20 runs for each test to average, but that was tying up my machine for well over an hour. I can do a higher number though if more precision is desired.

The 3 fields represent the current repo avg in ms, the patch avg in ms, and the mean improvement in ms.

functional/restartTests/testAddons_changeTheme/
30947,29563,1384
functional/restartTests/testAddons_enableDisableExtension/
37083,35323,1760
functional/restartTests/testAddons_installMultipleExtensions/
34732,28363,6369
functional/restartTests/testAddons_installTheme/
26681,25258,1423
functional/restartTests/testAddons_installUninstallHardBlocklistedExtension/
44669,39141,5528
functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/
46269,40046,6223
functional/restartTests/testAddons_pluginDisabledAfterRestart/
28551,23358,5193
functional/restartTests/testAddons_uninstallExtension/
44216,43074,1142
functional/restartTests/testAddons_uninstallTheme/
36835,33067,3768

If it looks reasonable I will proceed with refactoring the other intended tests.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

That looks awesome! Wow, I cannot believe that we really spend more than 30s in waiting for the discovery pane to be loaded. So yes, please lets get this killed in any place.

Comment 21

[Jonathan French (:jfrench)]

Will do, I will continue on. I think the numbers look more-dramatic due to my slow machine. I believe with the real test nodes the times will be shorter and the improvement deltas smaller.

I will re-run everything when done all the changes and produce another set of numbers for the patch. At least so there is something for others to compare against.

Now that I have my little run script it's relatively easy to do.

Comment 22

[Jonathan French (:jfrench)]

I've finished updating the remaining tests. Not a lot, there were about 8 more.

Question about two tests below. UpAndComing seems to rely on the discovery pane contents so I think is n/a. But installAddonWithEULA sets the addon category to extension after open, and I think can receive the about:home optimization. If you can confirm?

(n/a) remote/restartTests/testDiscoveryPane_UpAndComingModule/test1.js
(done) remote/restartTests/testDiscoveryPane_installAddonWithEULA/test1.js

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Every testDiscoveryPane test should not be converted. Those tests explicitly make use of the discovery pane. So you will have to revert the change for that file.

Comment 24

[Jonathan French (:jfrench)]

Great, thanks.

Comment 25

[Jonathan French (:jfrench)]

Here's the remaining six tests, with the same process of ten runs each.

Same as before, current repo avg in ms, the patch avg in ms, and the mean improvement in ms

tests/endurance/testAddons_OpenAndCloseGetAddonsPane/
16153,15395,758
tests/endurance/testAddons_OpenAndCloseExtensionList/
15656,15646,10
tests/remote/restartTests/testAddons_installFromFTP/
27797,26514,1283
tests/functional/testAddons/testManagerKeyboardShortcut.js
12615,12174,441
tests/functional/testAddons/testPluginDisableAffectsAboutPlugins.js
15360,14540,820
tests/remote/testAddons/testSearchAddons.js
31702,25222,6480

The last test I found after; it was using am.open(). I did a repo search for that also, and other than than the testDiscoveryPane tests which we aren't going to touch, it was the only occurrence.

Again I think the numbers on a real test node with a fast LAN, will be faster and the deltas smaller.

I could have missed something, but based on all the digging and pattern searches for all tests with 1) addon modules, 2) addonsManager.open() 3) am.open() 4) discoveryPane methods, and 5) manual review of any related tests in sibling folders, I think that may have covered them all.

I plan to put up a patch for code review if it makes sense at this point.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Absolutely! Lets go ahead. Those are great numbers and I would love to see those live in our CI and how it affects our turnaround times for those jobs. Thank you Jonathan!

Comment 27

[Jonathan French (:jfrench)]

Attachment: about home discovery pane (default) rev1.0

Patch "about home discovery pane (default) rev1.0" for default branch. Nineteen files modified.

Contains additions of setDiscoveryPaneURL(), resetDiscoveryPaneURL(), and removal of unused BASE_URL and TEST_DATA constants where applicable.

One test wasn't run through the timing evaluation since it is presently being skipped.
remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/
Although it was 'tested' just to ensure it ran fine as a skip, post change.

Tests changed in the patch are

Functional
----------
testAddons_changeTheme/
testAddons_enableDisableExtension/
testAddons_installMultipleExtensions/
testAddons_installTheme/
testAddons_installUninstallHardBlocklistedExtension/
testAddons_installUninstallSoftBlocklistedExtension/
testAddons_pluginDisabledAfterRestart/
testAddons_uninstallExtension/
testAddons_uninstallTheme

Endurance
---------
endurance/testAddons_OpenAndCloseExtensionList/
endurance/testAddons_OpenAndCloseGetAddonsPane/

Addons
------
functional/testAddons/testManagerKeyboardShortcut.js
functional/testAddons/testPluginDisableAffectsAboutPlugins.js

Remote
------
remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/
remote/restartTests/testAddons_installFromFTP/
remote/testAddons/testSearchAddons.js

Tested in:
Windows XP32 SP3
mozmill-env 1.5.24
default branch + patch rev1.0
latest Nightly 27.0a1 20131022030202 151550

Comment 28

[Jonathan French (:jfrench)]

(In reply to Jonathan French (:jfrench) from comment #25)

> tests/endurance/testAddons_OpenAndCloseExtensionList/
> 15656,15646,10

Looking at this test again, I am not sure it really requires the change. The negligible performance improvement seems to imply that also. It does open the addon manager, but it is lightning fast during the test since it is opening the extensions list and immediately closing it. I suppose if further updates are made to the test, it may be good to still have it in. I defer to Henrik on this.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 821005 [details] [diff] [review]
about home discovery pane (default) rev1.0

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

Overall this looks fine. Please see my inline comments to what would have to be updated.

Curiously I also did a full testrun_functional run and the differences of the duration is not that impressive. Actually not sure why that happens that those are only 17s. It should be more. As best we compare when it is live in CI.

before:
real	9m47.107s
user	1m50.316s
sys	0m14.836s

after:
real	9m30.844s
user	1m46.612s
sys	0m13.236s

::: tests/functional/restartTests/testAddons_enableDisableExtension/test2.js
@@ +13,5 @@
>  function setupModule(aModule) {
>    aModule.controller = mozmill.getBrowserController();
>  
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

Let do that in test1.js so that we prepare everything in the first test.

::: tests/functional/restartTests/testAddons_installMultipleExtensions/test2.js
@@ +18,2 @@
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

Same here. Please do this call in test1.js

::: tests/functional/restartTests/testAddons_installTheme/test2.js
@@ +15,5 @@
>  function setupModule(aModule) {
>    aModule.controller = mozmill.getBrowserController();
>  
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

Same here for test1.js

::: tests/functional/restartTests/testAddons_installUninstallHardBlocklistedExtension/test2.js
@@ +19,2 @@
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

In test1.js please.

::: tests/functional/restartTests/testAddons_installUninstallHardBlocklistedExtension/test4.js
@@ +24,5 @@
>    prefs.preferences.clearUserPref(PREF_UPDATE_EXTENSION);
>  
>    delete persisted.addon;
>    aModule.addonsManager.close();
> +  addons.resetDiscoveryPaneURL();

Given that we modify a preference here lets move it up before the prefs calls.

::: tests/functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/test2.js
@@ +20,2 @@
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

in test1.js please.

::: tests/functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/test4.js
@@ +24,5 @@
>    prefs.preferences.clearUserPref(PREF_UPDATE_EXTENSION);
>  
>    delete persisted.addon;
>    aModule.addonsManager.close();
> +  addons.resetDiscoveryPaneURL();

Move it up please.

::: tests/functional/restartTests/testAddons_pluginDisabledAfterRestart/test2.js
@@ +23,5 @@
>  }
>  
>  function teardownModule(aModule) {
>    // Enable the plugin that was disabled
>    addons.enableAddon(persisted.plugin.id);

Oh my. The above line should not be part of the teardown. If we need a reset call here it has to be an API call but not through the UI. Jonathan can you please file a new bug for that?

It might be that other tests have the same problem.

::: tests/functional/restartTests/testAddons_uninstallExtension/test2.js
@@ +14,5 @@
>  function setupModule(aModule) {
>    aModule.controller = mozmill.getBrowserController();
>  
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

In test1.js please.

::: tests/remote/restartTests/testAddons_installFromFTP/test2.js
@@ +12,5 @@
>  function setupModule(aModule) {
>    aModule.controller = mozmill.getBrowserController();
>  
>    aModule.addonsManager = new addons.AddonsManager(aModule.controller);
> +  addons.setDiscoveryPaneURL("about:home");

In test1.js please.

Comment 30

[Jonathan French (:jfrench)]

Thanks Henrik, I will make the above changes.

> Oh my. The above line should not be part of the teardown. If we need a reset call here it has to be
> an API call but not through the UI. Jonathan can you please file a new bug for that?
Entered as bug 930271.

Comment 31

[Jonathan French (:jfrench)]

(In reply to Henrik Skupin (:whimboo) from comment #29)
> >    prefs.preferences.clearUserPref(PREF_UPDATE_EXTENSION);
> >  
> >    delete persisted.addon;
> >    aModule.addonsManager.close();
> > +  addons.resetDiscoveryPaneURL();
> 
> Given that we modify a preference here lets move it up before the prefs
> calls.

There seems to be a lot of inconsistent ordering in the teardowns across the affected tests. My plan is to order all teardowns like this (including whitespace) - which is predominant order in the tests:

delete persisted.thing;

aModule.addonsManager.close();
addons.resetDiscoveryPaneURL();

prefs.preferences.thing

Sound good?...

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No, any backend API calls should come first. At least as long as we use a path through the UI to clean-up things. In this case addonsManager.close(). What we could do here for now is to specify true as parameter for calling this method. It will ignore failures, but there might still be cases when we can't close the manager. But that refactoring should happen on another bug.

Comment 33

[Jonathan French (:jfrench)]

Ok, I will leave that re-ordering of the teardowns outside the scope of this patch.

A side note, I did try it yesterday with affected tests, and all tests passed. The teardowns also did seem a lot easier to read test-to-test, when they were consistent.

Anyway, I will revert that change and put up a patch as planned.

Comment 34

[Jonathan French (:jfrench)]

Attachment: about home discovery pane (default) rev1.1

Patch "about home discovery pane (default) rev1.1" for default branch. Twenty five files modified.

Changes made per review comments in Comment 29.

Some files have been added to the patch as a result of the case by case transfer of about:home from test2's to test1. Where previously those test2's were untouched.

Tested in:
Windows XP32 SP3
mozmill-env 1.5.24
default branch + patch rev1.1
latest Nightly 27.0a1 20131025030239 152127

Comment 35

[Jonathan French (:jfrench)]

A report with latest Nightly also.

Functional
http://mozmill-crowd.blargon7.com/#/functional/report/a766a11d45054bd1713f757bc410a778
http://mozmill-crowd.blargon7.com/#/functional/report/a766a11d45054bd1713f757bc410aba9

Comment 36

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 822515 [details] [diff] [review]
about home discovery pane (default) rev1.1

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

We are close to get this landed. Just two nits to fix as far as I have seen. See my comments inline.

::: tests/functional/restartTests/testAddons_installMultipleExtensions/test2.js
@@ +20,5 @@
>  }
>  
>  function teardownModule(aModule) {
>    aModule.addonsManager.close();
> +  addons.resetDiscoveryPaneURL();

nit: please move the closing of the add-on manager down. Otherwise we could fail to reset the custom preferences as mentioned before. Might have been a look over.

::: tests/remote/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test2.js
@@ +24,5 @@
>  }
>  
>  function teardownModule(aModule) {
> +  aModule.addonsManager.close();
> +  addons.resetDiscoveryPaneURL();

Same here. Closing the add-on manager should be last.

Comment 37

[Jonathan French (:jfrench)]

Will do. It seems there are several more tests that come under this criteria also. I will do them all.

Comment 38

[Jonathan French (:jfrench)]

Actually these other tests (eg. changeTheme, others) while they fall under the same ordering criteria, they already had a resetDiscoveryPaneURL() and so weren't in the patch. So I assume if this ordering is desired on a larger scale it should be done in another bug. I will just do the two above.

Comment 39

[Jonathan French (:jfrench)]

Attachment: about home discovery pane (default) rev1.2

Patch "about home discovery pane (default) rev1.2" for default branch. Twenty five files modified.

Nits addressed as per Comment 36.

Tested in:
Windows XP32 SP3
mozmill-env 1.5.24
default branch + patch rev1.2
latest Nightly 27.0a1 20131028030205 152443

Comment 40

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Jonathan French (:jfrench) from comment #38)
> Actually these other tests (eg. changeTheme, others) while they fall under
> the same ordering criteria, they already had a resetDiscoveryPaneURL() and
> so weren't in the patch. So I assume if this ordering is desired on a larger
> scale it should be done in another bug. I will just do the two above.

Yeah, we should do that then. Please file an appropriate bug for it. I will mentor it. Thanks.

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 823391 [details] [diff] [review]
about home discovery pane (default) rev1.2

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

Looks great. Now I'm curious about the speed improvements we will see in our CI. I totally vote for backporting this fix down to esr24. Would you mind to check that please?

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/749c50885839

Comment 43

[Jonathan French (:jfrench)]

The patch pushes cleanly to aurora, beta, release, and esr-24(with fuzz on esr-24). All affected tests for each available branch-build pass, and behave as expected.

Teardown ordering bug is now bug 932347.

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Jonathan! I went ahead and backported it given that regressions were visible today:

http://hg.mozilla.org/qa/mozmill-tests/rev/b5a0a366d3e0 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/11db1b60fb1d (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/49a11ef6fcc5 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/c71d1778f2df (esr24)

Timings are varying a lot across different platforms and even between machines. We will have to observe it a bit more. But in CI we might not have a performance improvement larger than 30s or so. The connection is kinda too fast.