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 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 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 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 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 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 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 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 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 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 Remus Pop (:RemusPop) 2012-07-06 06:57:59 PDT
Strange enough, controller.window.location.host returns "browser".
Comment 10 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 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 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 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 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 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 Remus Pop (:RemusPop) 2012-07-12 05:19:56 PDT
Created attachment 641430 [details] [diff] [review]
patch v2 (esr10) [checked-in]
Comment 17 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 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 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.