Last Comment Bug 767821 - Failure in testRemoveAllCookies.js | Timeout waiting for page loaded
: Failure in testRemoveAllCookies.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:09 PDT by Anthony Hughes (:ashughes) [GFX][QA][Mentor]
Modified: 2012-07-09 16:12 PDT (History)
2 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed
fixed
fixed


Attachments
patch v1 (beta) (3.47 KB, patch)
2012-07-03 00:51 PDT, Remus Pop (:RemusPop)
dave.hunt: review-
Details | Diff | Splinter Review
patch v2 (beta) (3.72 KB, patch)
2012-07-04 05:50 PDT, Remus Pop (:RemusPop)
hskupin: review-
Details | Diff | Splinter Review
patch v3 (beta) (3.73 KB, patch)
2012-07-04 06:39 PDT, Remus Pop (:RemusPop)
dave.hunt: review+
Details | Diff | Splinter Review
patch v3 (esr10) (5.52 KB, patch)
2012-07-06 07:37 PDT, Remus Pop (:RemusPop)
dave.hunt: review-
Details | Diff | Splinter Review
patch v3 (esr10) (5.52 KB, patch)
2012-07-09 05:44 PDT, Remus Pop (:RemusPop)
no flags Details | Diff | Splinter Review
patch v3 (esr10) (3.57 KB, patch)
2012-07-09 07:04 PDT, Remus Pop (:RemusPop)
hskupin: review+
Details | Diff | Splinter Review

Description Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-24 12:09:48 PDT
Failure discovered while running 14.0b9 functional tests.

Module: /testCookies/testRemoveAllCookies.js 	
Test: testRemoveAllCookies.js::testRemoveAllCookies 	
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 03:50:30 PDT
Remus, please make this test using mozqa.com. There is a cookie testcase available.
Comment 2 Remus Pop (:RemusPop) 2012-07-03 00:51:20 PDT
Created attachment 638612 [details] [diff] [review]
patch v1 (beta)

Used the local cookie page and removed gTimeout constant from the wait calls.
Comment 3 Dave Hunt (:davehunt) 2012-07-04 03:48:18 PDT
Comment on attachment 638612 [details] [diff] [review]
patch v1 (beta)

Taking this for review.
Comment 4 Dave Hunt (:davehunt) 2012-07-04 04:06:39 PDT
Comment on attachment 638612 [details] [diff] [review]
patch v1 (beta)

> /**
>  * Test removing all cookies via the cookie manager
>  */
> var testRemoveAllCookies = function() {
>-  // Go to mozilla.org to build a list of cookies
>-  controller.open("http://www.mozilla.org/");
>+  // Go to google.com to build a list of cookies
>+  controller.open(LOCAL_TEST_PAGE);

This comment referred to the page originally being opened, which is now a local test page and not google.com

>   controller.waitForPageLoad();
>-
>+  
>   controller.open("http://www.google.com/");
>   controller.waitForPageLoad();

As we're making this page use local test pages, can we not remove the external dependency on google.com? I'm guessing that this is here so that we have cookies from multiple domains.
Comment 5 Dave Hunt (:davehunt) 2012-07-04 04:29:16 PDT
Speaking to Henrik on IRC, you should use mozqa.com with different domains. For example:

http://domain1.mozqa.com/data/firefox/cookies/cookie_single.html
http://domain2.mozqa.com/data/firefox/cookies/cookie_single.html

This maintains the integrity of the test but uses the much more lightweight test pages. In the future we should be able to use local test pages by faking fully qualified domain names, but for now we must use remote pages.
Comment 6 Remus Pop (:RemusPop) 2012-07-04 05:50:55 PDT
Created attachment 639065 [details] [diff] [review]
patch v2 (beta)

I've created an array for the different domains and changed the comment.
Comment 7 Henrik Skupin (:whimboo) 2012-07-04 06:06:27 PDT
Comment on attachment 639065 [details] [diff] [review]
patch v2 (beta)

>+const COOKIE_PAGE = "/data/firefox/cookies/cookie_single.html";
>+const DOMAINS = ["http://domain1.mozqa.com",
>+                 "http://domain2.mozqa.com"];

Don't invent new names. Stick with those constants we are making use of in all the other tests.
Comment 8 Remus Pop (:RemusPop) 2012-07-04 06:39:23 PDT
Created attachment 639081 [details] [diff] [review]
patch v3 (beta)

Addressed all requests.
Comment 9 Dave Hunt (:davehunt) 2012-07-04 07:09:18 PDT
Comment on attachment 639081 [details] [diff] [review]
patch v3 (beta)

Looks good. Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/11ebce9d12a6 (default)
Comment 10 Henrik Skupin (:whimboo) 2012-07-05 07:07:26 PDT
Landed on other brnaches except esr10 which needs another patch:

http://hg.mozilla.org/qa/mozmill-tests/rev/4e64a5b63983 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/faf967b16b2a (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/70645719888d (release)
Comment 11 Remus Pop (:RemusPop) 2012-07-06 07:37:11 PDT
Created attachment 639660 [details] [diff] [review]
patch v3 (esr10)

Updated patch so it applies cleanly in esr10.
Comment 12 Dave Hunt (:davehunt) 2012-07-09 03:05:54 PDT
Comment on attachment 639660 [details] [diff] [review]
patch v3 (esr10)

I don't think we're changing the license blocks for mozilla-esr10. Could you revert that and then this will look good to me.
Comment 13 Remus Pop (:RemusPop) 2012-07-09 05:44:52 PDT
Created attachment 640190 [details] [diff] [review]
patch v3 (esr10)

Reverted the old license block.
Comment 14 Dave Hunt (:davehunt) 2012-07-09 06:50:06 PDT
Did you forget to qref? The license block change is still there...
Comment 15 Remus Pop (:RemusPop) 2012-07-09 07:04:48 PDT
Created attachment 640202 [details] [diff] [review]
patch v3 (esr10)

Yes, I somehow omitted the refresh.
Comment 16 Henrik Skupin (:whimboo) 2012-07-09 16:12:28 PDT
Pushed to esr10 branch:
http://hg.mozilla.org/qa/mozmill-tests/rev/2badd58a6068

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