767821 - Failure in testRemoveAllCookies.js | Timeout waiting for page loaded

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

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][⌚️UTC+1]]

Remus, please make this test using mozqa.com. There is a cookie testcase available.

Comment 2

[Remus Pop (:RemusPop)]

Attachment: patch v1 (beta)

Used the local cookie page and removed gTimeout constant from the wait calls.

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 638612 [details] [diff] [review]
patch v1 (beta)

Taking this for review.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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] [he/him] ⌚BST]

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)]

Attachment: patch v2 (beta)

I've created an array for the different domains and changed the comment.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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)]

Attachment: patch v3 (beta)

Addressed all requests.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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][⌚️UTC+1]]

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)]

Attachment: patch v3 (esr10)

Updated patch so it applies cleanly in esr10.

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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)]

Attachment: patch v3 (esr10)

Reverted the old license block.

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Did you forget to qref? The license block change is still there...

Comment 15

[Remus Pop (:RemusPop)]

Attachment: patch v3 (esr10)

Yes, I somehow omitted the refresh.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Pushed to esr10 branch:
http://hg.mozilla.org/qa/mozmill-tests/rev/2badd58a6068