Failure in testRemoveAllCookies.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

(2 attachments, 4 obsolete attachments)

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
status-firefox14: --- → affected
Remus, please make this test using mozqa.com. There is a cookie testcase available.
Assignee: nobody → remus.pop
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 638612 [details] [diff] [review]
patch v1 (beta)

Used the local cookie page and removed gTimeout constant from the wait calls.
Attachment #638612 - Flags: review?(hskupin)
Attachment #638612 - Flags: review?(dave.hunt)
Comment on attachment 638612 [details] [diff] [review]
patch v1 (beta)

Taking this for review.
Attachment #638612 - Flags: review?(hskupin)
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.
Attachment #638612 - Flags: review?(dave.hunt) → review-
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.
(Assignee)

Comment 6

5 years ago
Created attachment 639065 [details] [diff] [review]
patch v2 (beta)

I've created an array for the different domains and changed the comment.
Attachment #638612 - Attachment is obsolete: true
Attachment #639065 - Flags: review?(dave.hunt)
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.
Attachment #639065 - Flags: review?(dave.hunt) → review-
(Assignee)

Comment 8

5 years ago
Created attachment 639081 [details] [diff] [review]
patch v3 (beta)

Addressed all requests.
Attachment #639065 - Attachment is obsolete: true
Attachment #639081 - Flags: review?(hskupin)
Attachment #639081 - Flags: review?(dave.hunt)
Comment on attachment 639081 [details] [diff] [review]
patch v3 (beta)

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

Updated

5 years ago
status-firefox-esr10: --- → affected
status-firefox13: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
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)
status-firefox13: affected → fixed
status-firefox14: affected → fixed
status-firefox15: affected → fixed
(Assignee)

Comment 11

5 years ago
Created attachment 639660 [details] [diff] [review]
patch v3 (esr10)

Updated patch so it applies cleanly in esr10.
Attachment #639660 - Flags: review?(hskupin)
Attachment #639660 - Flags: review?(dave.hunt)
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.
Attachment #639660 - Flags: review?(hskupin)
Attachment #639660 - Flags: review?(dave.hunt)
Attachment #639660 - Flags: review-
(Assignee)

Comment 13

5 years ago
Created attachment 640190 [details] [diff] [review]
patch v3 (esr10)

Reverted the old license block.
Attachment #639660 - Attachment is obsolete: true
Attachment #640190 - Flags: review?(dave.hunt)
Did you forget to qref? The license block change is still there...
(Assignee)

Comment 15

5 years ago
Created attachment 640202 [details] [diff] [review]
patch v3 (esr10)

Yes, I somehow omitted the refresh.
Attachment #640190 - Attachment is obsolete: true
Attachment #640190 - Flags: review?(dave.hunt)
Attachment #640202 - Flags: review?(dave.hunt)
Attachment #640202 - Flags: review?(dave.hunt) → review+
Pushed to esr10 branch:
http://hg.mozilla.org/qa/mozmill-tests/rev/2badd58a6068
status-firefox-esr10: affected → fixed
You need to log in before you can comment on or make changes to this bug.