Last Comment Bug 767822 - Failure in testGreyLarry.js | Timeout waiting for page loaded
: Failure in testGreyLarry.js | Timeout waiting for page loaded
Status: RESOLVED FIXED
[mozmill-test-failure]
:
Product: Mozilla QA
Classification: Other
Component: Mozmill Tests (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Remus Pop (:RemusPop)
:
:
Mentors:
http://mozmill-ci.blargon7.com/#/func...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-24 12:12 PDT by Anthony Hughes (:ashughes) [GFX][QA][Mentor]
Modified: 2012-07-12 10:07 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed
fixed


Attachments
patch v1 (default, aurora, beta) (2.63 KB, patch)
2012-06-26 02:01 PDT, Remus Pop (:RemusPop)
hskupin: review-
Details | Diff | Splinter Review
patch v2 (default) [checked-in] (3.56 KB, patch)
2012-07-06 04:57 PDT, Remus Pop (:RemusPop)
dave.hunt: review+
Details | Diff | Splinter Review
patch v3 (default, aurora, beta) (2.31 KB, patch)
2012-07-10 07:30 PDT, Remus Pop (:RemusPop)
no flags Details | Diff | Splinter Review
patch v2 (release) [checked-in] (3.88 KB, patch)
2012-07-12 04:57 PDT, Remus Pop (:RemusPop)
dave.hunt: review+
Details | Diff | Splinter Review
patch v2 (esr10) [checked-in] (4.16 KB, patch)
2012-07-12 05:19 PDT, Remus Pop (:RemusPop)
dave.hunt: review+
Details | Diff | Splinter Review

Description User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-24 12:12:15 PDT
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
Comment 1 User image Henrik Skupin (:whimboo) 2012-06-25 02:47:41 PDT
This can happen on each branch. Remus, can you please make this test local?
Comment 2 User image Remus Pop (:RemusPop) 2012-06-26 02:01:53 PDT
Created attachment 636632 [details] [diff] [review]
patch v1 (default, aurora, beta)

Now we use a local test page.
Comment 3 User image Henrik Skupin (:whimboo) 2012-06-26 02:08:47 PDT
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.
Comment 4 User image Remus Pop (:RemusPop) 2012-06-27 07:28:37 PDT
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.
Comment 5 User image Henrik Skupin (:whimboo) 2012-06-27 07:32:11 PDT
With match you wouldn't have to call 'split("/")[2];' and more traceable output would exist in the assertion message.
Comment 6 User image Remus Pop (:RemusPop) 2012-07-06 04:57:30 PDT
Created attachment 639627 [details] [diff] [review]
patch v2 (default) [checked-in]

Addressed all requests.
Comment 7 User image Dave Hunt (:davehunt) 2012-07-06 06:01:40 PDT
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)
Comment 8 User image Henrik Skupin (:whimboo) 2012-07-06 06:05:14 PDT
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.
Comment 9 User image Remus Pop (:RemusPop) 2012-07-06 06:57:59 PDT
Strange enough, controller.window.location.host returns "browser".
Comment 10 User image Henrik Skupin (:whimboo) 2012-07-09 16:09:12 PDT
You probably want 'controller.window.content.location.host' or try to get it via the content document.
Comment 11 User image Remus Pop (:RemusPop) 2012-07-10 07:30:55 PDT
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.
Comment 12 User image Dave Hunt (:davehunt) 2012-07-11 02:29:42 PDT
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.
Comment 13 User image Henrik Skupin (:whimboo) 2012-07-11 06:26:20 PDT
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.
Comment 14 User image Remus Pop (:RemusPop) 2012-07-12 04:40:37 PDT
Applies cleanly on aurora and beta. Release and esr10 have to be fixed in regards to the favicon.
Comment 15 User image Remus Pop (:RemusPop) 2012-07-12 04:57:23 PDT
Created attachment 641428 [details] [diff] [review]
patch v2 (release) [checked-in]

This needed the fix for favicon.
Comment 16 User image Remus Pop (:RemusPop) 2012-07-12 05:19:56 PDT
Created attachment 641430 [details] [diff] [review]
patch v2 (esr10) [checked-in]
Comment 17 User image Dave Hunt (:davehunt) 2012-07-12 09:56:20 PDT
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)
Comment 18 User image Dave Hunt (:davehunt) 2012-07-12 10:01:58 PDT
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)
Comment 19 User image Dave Hunt (:davehunt) 2012-07-12 10:06:34 PDT
Comment on attachment 641430 [details] [diff] [review]
patch v2 (esr10) [checked-in]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/27997d33fec6 (esr10)

Note You need to log in before you can comment on or make changes to this bug.