585757 - Make Mozmill-test testRemoveCookie.js local

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Module: testCookies/testRemoveCookie.js
Test-page: test-files/cookies/cookie_single.html

Comment 1

[Aaron Train [:aaronmt]]

Attachment: Patch v1 - (default)

cleanup + converts test to make use of local-data

Comment 2

[u279076]

Comment on attachment 467008 [details] [diff] [review]
Patch v1 - (default)

Looks good, r+.  Over to Henrik for additional review.

Comment 3

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

Comment on attachment 467008 [details] [diff] [review]
Patch v1 - (default)

>+var setupModule = function()
> {

Please fix the bracing for all functions.

>+  cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);

Please always move down calls to methods of objects into the next line. 

With those changes you can take over r=me.

Comment 4

[Aaron Train [:aaronmt]]

Attachment: Patch v1.1 - (default)

fixes from comment above

Comment 5

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

Comment on attachment 471502 [details] [diff] [review]
Patch v1.1 - (default)

>-  module.cm = Cc["@mozilla.org/cookiemanager;1"].getService(Ci.nsICookieManager2);
>+  cm = Cc["@mozilla.org/cookiemanager;1"]
>+          .getService(Ci.nsICookieManager2);

It's a nit bug please use:

>+  cm = Cc["@mozilla.org/cookiemanager;1"].
>+       getService(Ci.nsICookieManager2);

Comment 6

[Aaron Train [:aaronmt]]

Attachment: Patch v1.1 [indent fix] - (default)

Comment 7

[u279076]

(In reply to comment #6)
> Created attachment 471567 [details] [diff] [review]
> Patch v1.1 [indent fix] - (default)

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/152fe346f198

Please follow up with 1.9.2 and 1.9.1 patches as appropriate.

Comment 8

[Aaron Train [:aaronmt]]

Attachment: Patch v1.1 - (1.9.2)

Comment 9

[u279076]

(In reply to comment #8)
> Created attachment 473675 [details] [diff] [review]
> Patch v1.1 - (1.9.2)

Landed on mozilla1.9.2:
http://hg.mozilla.org/qa/mozmill-tests/rev/1beb92ab829e

Comment 10

[Aaron Train [:aaronmt]]

Attachment: Patch v1.1 - (1.9.1)

Comment 11

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

Please revise the patches for default and mozmill1.9.2 so those do not test cookies for .mozilla.org. We need follow-up patches.

Comment 12

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (default) [localhost fix]

s/.mozilla.org/localhost

Comment 13

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (1.9.2) [localhost fix]

1.9.2

Comment 14

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (default) [localhost fix]

Forgot to change the name: litmus_1

Comment 15

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (1.9.2) [localhost fix]

Comment 16

[Aaron Train [:aaronmt]]

Comment on attachment 475966 [details] [diff] [review]
Patch v1.2 - (default) [localhost fix]

Path should be /cookies/

Comment 17

[Aaron Train [:aaronmt]]

Comment on attachment 475967 [details] [diff] [review]
Patch v1.2 - (1.9.2) [localhost fix]

Path should be /cookies/

Comment 18

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (default)

+ added the persistence object as per Henrik's suggestion in the other cookie bugs
+ proper path of cookie is /cookies/

Comment 19

[Aaron Train [:aaronmt]]

Attachment: Patch v1.2 - (1.9.2)

Comment 20

[u279076]

(In reply to comment #19)
> Created attachment 477728 [details] [diff] [review]
> Patch v1.2 - (1.9.2)

Diff between two patches is essentially the same (diff on comments).  An r+ on the default patch will be assumed for this patch as well.

Comment 21

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

Comment on attachment 477727 [details] [diff] [review]
Patch v1.2 - (default)

>+  persisted.hostName = {};

undefined please.

>-  var removed = !cm.cookieExists({host: ".mozilla.org", name: "__utmz", path: "/"});
>+  var removed = !cm.cookieExists({host: persisted.hostName, 
>+                                  name: "litmus_1", 
>+                                  path: "/cookies/"});

Please update the indentation:

>+  var removed = !cm.cookieExists({
>+                  host: persisted.hostName, 
>+                  name: "litmus_1", 

>   controller.assertJS("subject.list.view.rowCount == subject.numberCookies",
>-                        {list: cookiesList, numberCookies: origNumCookies - 1});
>+                      {list: cookiesList, 
>+                       numberCookies: origNumCookies - 1});

Same here.

With those changes r=me.

Comment 22

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

Comment on attachment 477728 [details] [diff] [review]
Patch v1.2 - (1.9.2)

This patch also needs the requested updates as the default one.

Comment 23

[Aaron Train [:aaronmt]]

Attachment: Patch v1.3 - (default) + [indent fix]

Comment 24

[Aaron Train [:aaronmt]]

Attachment: Patch v1.3 - (1.9.2) + [indent fix]

Comment 25

[Aaron Train [:aaronmt]]

Attachment: Patch v1.3 - (1.9.1) + [indent fix]

Looks like the initial changes never landed on 1.9.1 as the patch wouldn't apply. Requesting review to play it safe

Comment 26

[u279076]

(In reply to comment #23)
> Created attachment 478294 [details] [diff] [review]
> Patch v1.3 - (default) + [indent fix]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/30e5398131dd [default]

Comment 27

[u279076]

(In reply to comment #24)
> Created attachment 478295 [details] [diff] [review]
> Patch v1.3 - (1.9.2) + [indent fix]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e81f9f5e17c4 [mozilla1.9.2]

Comment 28

[u279076]

(In reply to comment #25)
> Created attachment 478296 [details] [diff] [review]
> Patch v1.3 - (1.9.1) + [indent fix]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/996a6e0599f0 [mozilla1.9.1]

Comment 29

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

Move of Mozmill Test related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.