987565 - Test failure "controller.waitForPageLoad" in testBookmarks_OpenAndClose/test1.js and testBookmarks_OpenAllInTabs/test1.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Daniel Gherasim]

This happened on Mac OSX 10.7 (mm-osx-107-2). 
Log says it failed loading about:newtab and the readyState it's marked as complete.

17:19:14 TEST-START | testBookmarks_OpenAndClose/test1.js | setupModule
17:19:17 TEST-START | testBookmarks_OpenAndClose/test1.js | testOpenAndCloseAllBookmarks
17:21:24 ERROR | Test Failure | {
17:21:24   "exception": {
17:21:24     "message": "controller.waitForPageLoad(URI=about:newtab, readyState=complete)", 
17:21:24     "lineNumber": 27, 
17:21:24     "name": "AssertionError", 
17:21:24     "fileName": "resource://mozmill/modules/errors.js"
17:21:24   }
17:21:24 }
17:21:24 TEST-UNEXPECTED-FAIL | testBookmarks_OpenAndClose/test1.js | testOpenAndCloseAllBookmarks
17:21:24 TEST-START | testBookmarks_OpenAndClose/test1.js | teardownModule

Comment 1

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

So what are we going to load in that line? Also I miss the link to the dashboard failure list for this kind of failure. Which branches are affected?

Comment 2

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

Daniel please check all the open waitForPageLoad() bugs for the about:newtab failure. There should be a single bug only for that issue.

Comment 3

[Daniel Gherasim]

We have a more general waitForPageLoad() bug where we tracked all this types of failures and other test specific bugs with no information about the page that failed.
Now that we have more information about each call, we can group this failures by the page that failed loading, like about:newtab or about:blank, or a remote page.

I suggest we name this bug 'Test failure "controller.waitForPageLoad()" waiting for 'about:newtab' page'.

Henrik, would that be a good way of tracking this failures in the future ?

Comment 4

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

We might only want to differentiate between remote pages, local pages, and self-served (about:) pages. So lets create check all existing bugs for about: and mark those as dupe of the bug first created for it.

Comment 5

[Andreea Matei [:AndreeaMatei]]

I reproduced this last week while testing 2.0.7, we should check continue on it this week.
http://mozmill-daily.blargon7.com/#/endurance/reports

Comment 6

[Andreea Matei [:AndreeaMatei]]

Fails on production too every time with Nightly only, raising priority, adding a skip patch in a moment.

Comment 7

[Andreea Matei [:AndreeaMatei]]

Attachment: skip.patch

Adding skip patch. I will update the summary here as this bug was initially created for testBookmarks_OpenAndClose with the same failure.
testBookmarks_OpenAllInTabs started to fail on September 11th, will investigate further.

Comment 8

[Andrei Eftimie]

Comment on attachment 8489320 [details] [diff] [review]
skip.patch

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

Interestingly I can't reproduce this at all on my machine.
This does fail at all on CI.

Disabled:
https://hg.mozilla.org/qa/mozmill-tests/rev/d947a65ee957 (default)

Comment 9

[Andreea Matei [:AndreeaMatei]]

The issue here is that we have the new settings button on the right side, regarding tiles options when we open a new tab. The fix needs this notification to be turned off, by setting browser.newtabpage.introShown pref to true. I'll file a bug for it. I tested it by setting the pref in the test itself.

Comment 10

[Andrei Eftimie]

I'm not sure a new bug is warranted. We should disable it here at the same time as we unskip the test.
ATM if we want it globally we need to insert it into mozprofile, and that's a bit over-the-top.

I've seen that notification locally lately, but it hasn't impacted any of the testruns I've run.

Comment 11

[Andreea Matei [:AndreeaMatei]]

Sounds good to me, I was thinking to add it to mozprofile. But we do have only one test affected.. maybe add it in the tabs lib instead of the test, since that appears the first time you open about:newtab.

Comment 12

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

We should stop adding preferences to mozprofile. Especially those which only affect us in Mozmill, but not users of mozrunner. We can only add those to mozmill as I have lately done for e10s.

Comment 13

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1

Enabling test and setting the pref in closeAllTabs(). This is affecting only nightly.
http://mozmill-crowd.blargon7.com/#/endurance/report/f11964362bbc85ebfbc5b36de767ba8a

Ran also functional and remote on OSX to make sure we don't affect other tests.

Comment 14

[Andrei Eftimie]

Comment on attachment 8495189 [details] [diff] [review]
patch v1

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

::: firefox/lib/tabs.js
@@ +56,5 @@
>   *        MozMillController of the window to operate on
>   */
>  function closeAllTabs(aController)
>  {
> +  prefs.preferences.setPref(PREF_NEWTAB_INTRO, true);

Please add this in the tabBrowser constructor so it will always apply.
File a bug against mozmill to get this pref directly in as Henrik pointed out (see bug 1065436 attachment 8487894 [details] [diff] [review]),
and add a comment referencing that bug as usual when we add a workaround so we know when to safely remove it.

Comment 15

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2

Updated as requested.

Comment 16

[Andrei Eftimie]

Comment on attachment 8498875 [details] [diff] [review]
patch v2

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

Changed the commit message slightly.

Landed:
https://hg.mozilla.org/qa/mozmill-tests/rev/728f48e975df (default)

Comment 17

[Andreea Matei [:AndreeaMatei]]

This is still failing for  testBookmarks_OpenAllInTabs/test1.js, not sure why the pref isn't working as it should. I think it did for a while, then started to fail again. Will have to look further.

Comment 18

[Andrei Eftimie]

(In reply to Andreea Matei [:AndreeaMatei] from comment #17)
> This is still failing for  testBookmarks_OpenAllInTabs/test1.js, not sure
> why the pref isn't working as it should. I think it did for a while, then
> started to fail again. Will have to look further.

I don't see the Intro panel, so the preference is indeed working fine.
What I do see is the e10s notification panel (test manages to pass for me locally, but I did experience failures because of it in other places recently).

If it is indeed the issue, it will get fixed by mozmill 2.0.9

Comment 19

[Andrei Eftimie]

This should have been a new bug since it's a different issue. The original issue has been fixed on 2014-10-03 and we had no failures new failures in 2 weeks.

Lets leave it for now. The new issue has a high failure rate, so we should disable the test again.
Linux and WindowsXP don't seem to be affected. OSX and all other Win versions are failing at close to 100% rate.

I've relanded the initial skip patch:
https://hg.mozilla.org/qa/mozmill-tests/rev/e1af80200231 (default)

Comment 20

[Andreea Matei [:AndreeaMatei]]

This was failing due to the e10s popup, I tested with 2.0.9 env and it's passing now. So we'll wait for that in production before re-enabling this test.

Comment 21

[Andrei Eftimie]

With mozmill 2.0.9 all is green.

We have again all endurance tests running!
> RESULTS | Passed: 19
> RESULTS | Failed: 0
> RESULTS | Skipped: 0

Backed out skip:
https://hg.mozilla.org/qa/mozmill-tests/rev/492178336ac8 (default)