The default bug view has changed. See this FAQ.

Failure in testGreyLarry.js | Timeout waiting for page loaded

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ashughes, Assigned: RemusPop)

Tracking

unspecified

Firefox Tracking Flags

(firefox13 fixed, firefox14 fixed, firefox15 fixed, firefox16 fixed, firefox-esr10 fixed)

Details

(Whiteboard: [mozmill-test-failure], URL)

Attachments

(3 attachments, 2 obsolete attachments)

Failure discovered during Firefox 14.0b9 functional tests.

Module: /testSecurity/testGreyLarry.js 	
Test: testGreyLarry.js::testLarryGrey 	
Error: controller.waitForPageLoad(): Timeout waiting for page loaded
Report: http://mozmill-ci.blargon7.com/#/functional/report/44fc451c2e6b66b62172f2e13e16a438
status-firefox14: --- → affected
This can happen on each branch. Remus, can you please make this test local?
status-firefox-esr10: --- → affected
status-firefox13: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
(Assignee)

Updated

5 years ago
(Assignee)

Comment 2

5 years ago
Created attachment 636632 [details] [diff] [review]
patch v1 (default, aurora, beta)

Now we use a local test page.
Assignee: nobody → remus.pop
Status: NEW → ASSIGNED
Attachment #636632 - Flags: review?(hskupin)
Comment on attachment 636632 [details] [diff] [review]
patch v1 (default, aurora, beta)

> /**
>  * Litmus Test 8806: Grey Larry
>  */

Can you please remove that part? We should really get rid of it at the beginning of the file.

>+  var domain = LOCAL_TEST_FOLDER.split("/")[2];
>+  controller.assertValue(webIDDomainLabel, domain);
[..]
>   controller.assertValue(webIDOwnerLabel, securityOwner);

Can you please change those asserts into expect.* calls? For the former I would propose expect.match() with a regular expression which check for the domain and port at the beginning of the URI.
Attachment #636632 - Flags: review?(hskupin) → review-
(Assignee)

Comment 4

5 years ago
I don't see how .match will help us more then equal. We only must check if the accessed domain is the same as in the grey larry info page.
With match you wouldn't have to call 'split("/")[2];' and more traceable output would exist in the assertion message.
(Assignee)

Comment 6

5 years ago
Created attachment 639627 [details] [diff] [review]
patch v2 (default) [checked-in]

Addressed all requests.
Attachment #636632 - Attachment is obsolete: true
Attachment #639627 - Flags: review?(hskupin)
Attachment #639627 - Flags: review?(dave.hunt)
Comment on attachment 639627 [details] [diff] [review]
patch v2 (default) [checked-in]

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

Looks good. Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/d8c84efcec22 (default)
Attachment #639627 - Flags: review?(hskupin)
Attachment #639627 - Flags: review?(dave.hunt)
Attachment #639627 - Flags: review+

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox16: affected → fixed
Resolution: --- → FIXED
Comment on attachment 639627 [details] [diff] [review]
patch v2 (default) [checked-in]

>-  controller.assertValue(webIDDomainLabel, "www.mozilla.org");
>+  expect.match(webIDDomainLabel.getNode().value, /\/\/(.*[^\/])/.exec(LOCAL_TEST_FOLDER)[1],
>+               "The domain label should equal the domain");

Just thought about it and we should better have used 'window.location.host' here. No strange regex would have to be used in this case.
(Assignee)

Comment 9

5 years ago
Strange enough, controller.window.location.host returns "browser".
You probably want 'controller.window.content.location.host' or try to get it via the content document.
(Assignee)

Comment 11

5 years ago
Created attachment 640598 [details] [diff] [review]
patch v3 (default, aurora, beta)

Henrik thank you for the tip. The problem is I can only send the value from controller.window.content.location.host if using a global variable. Otherwise that is not defined in the window that opens.
Attachment #640598 - Flags: review?(hskupin)
Attachment #640598 - Flags: review?(dave.hunt)
Comment on attachment 640598 [details] [diff] [review]
patch v3 (default, aurora, beta)

I'd like to see what Henrik thinks of this change. I'm not keen on storing the host as a variable, but other than leaving the regex I'm not sure what else to propose.
Attachment #640598 - Flags: review?(dave.hunt)
Comment on attachment 640598 [details] [diff] [review]
patch v3 (default, aurora, beta)

Hm, haven't thought about that this check is in a callback. So keeping the regex would make sense. Lets discard my last idea and get former patch backported to other affected branches. Remus, please check if it applies cleanly.
Attachment #640598 - Attachment is obsolete: true
Attachment #640598 - Flags: review?(hskupin)
(Assignee)

Updated

5 years ago
Attachment #639627 - Attachment description: patch v2 (default, aurora, beta) → patch v2 (default, aurora, beta) [checked-in]
(Assignee)

Comment 14

5 years ago
Applies cleanly on aurora and beta. Release and esr10 have to be fixed in regards to the favicon.
(Assignee)

Updated

5 years ago
Attachment #639627 - Attachment description: patch v2 (default, aurora, beta) [checked-in] → patch v2 (default) [checked-in]
(Assignee)

Comment 15

5 years ago
Created attachment 641428 [details] [diff] [review]
patch v2 (release) [checked-in]

This needed the fix for favicon.
Attachment #641428 - Flags: review?(dave.hunt)
(Assignee)

Comment 16

5 years ago
Created attachment 641430 [details] [diff] [review]
patch v2 (esr10) [checked-in]
Attachment #641430 - Flags: review?(dave.hunt)
Please add a comment with a link when pushing changes.

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/d8c84efcec22 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/54915147af74 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/e6f59f1872f4 (beta)

Updated

5 years ago
status-firefox14: affected → fixed
status-firefox15: affected → fixed
Comment on attachment 641428 [details] [diff] [review]
patch v2 (release) [checked-in]

Check your commit messages have the correct reviewer details.

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/cfea1b7916a9 (release)
Attachment #641428 - Flags: review?(dave.hunt) → review+

Updated

5 years ago
status-firefox13: affected → fixed
Comment on attachment 641430 [details] [diff] [review]
patch v2 (esr10) [checked-in]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/27997d33fec6 (esr10)
Attachment #641430 - Attachment description: patch v2 (esr10) → patch v2 (esr10) [checked-in]
Attachment #641430 - Flags: review?(dave.hunt) → review+

Updated

5 years ago
Attachment #641428 - Attachment description: patch v2 (release) → patch v2 (release) [checked-in]

Updated

5 years ago
status-firefox-esr10: affected → fixed
You need to log in before you can comment on or make changes to this bug.