Test failure "The cookie has been removed - got 'false'" in /testPreferences/testRemoveCookie.js

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
P3
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Andrei Eftimie, Assigned: Teodor Druta, Mentored)

Tracking

unspecified

Firefox Tracking Flags

(firefox31 wontfix, firefox32 wontfix, firefox33 fixed, firefox34 fixed, firefox35 fixed, firefox36 fixed, firefox-esr24 wontfix, firefox-esr31 fixed)

Details

(Whiteboard: [mozmill-test-failure][lang=js], URL)

Attachments

(2 attachments, 1 obsolete attachment)

1.90 KB, patch
AndreeaMatei
: review+
Details | Diff | Splinter Review
1.61 KB, patch
Andrei Eftimie
: review+
Andrei Eftimie
: checkin+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
We have 1 failure on Aurora, Windows 7, fr:
http://mozmill-ci.blargon7.com/#/functional/report/25ad365ca7bcf4905e9b700b4f4b6135
(Reporter)

Updated

5 years ago
status-firefox21: --- → affected
Priority: -- → P3
Whiteboard: [mozmill-test-failure]

Comment 1

5 years ago
Did not happen for a couple of months so closing as WFM
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME

Comment 2

4 years ago
Happened again today on Linux Ubuntu 12.04 (x86_64) with Firefox 24.0 (24.0, de, 20130902131354)
http://mozmill-release.blargon7.com/#/functional/report/3c3cd991de01b704f3f65783a1c5d1d9
Status: RESOLVED → REOPENED
status-firefox24: --- → affected
OS: Windows 7 → All
Resolution: WORKSFORME → ---
(Reporter)

Updated

4 years ago
status-firefox23: --- → affected
Hardware: x86_64 → All
It failed again on Mac x64 with Beta es-ES
http://mozmill-release.blargon7.com/#/functional/report/6d6f6a58b02eeffc06eafa8beacd541a
status-firefox25: --- → affected
Happened on Ubuntu 13.04, with Beta ru:
http://mozmill-release.blargon7.com/#/functional/report/f54ca65f73fd56845c30b7ab7d392d72
(Reporter)

Comment 5

4 years ago
One new failure with 25 Release Candidate:
http://mozmill-release.blargon7.com/#/functional/report/f54ca65f73fd56845c30b7ab7dfda62a
status-firefox21: affected → wontfix
status-firefox23: affected → wontfix
status-firefox26: --- → ?
status-firefox27: --- → ?
status-firefox-esr17: --- → ?
status-firefox-esr24: --- → affected
Versions 21 and 23 don't exist anymore. Same for 24.0. For esr17 I would vote wontfix given that the branch is closed soon.
status-firefox21: wontfix → ---
status-firefox23: wontfix → ---
status-firefox24: affected → ---
status-firefox-esr17: ? → wontfix
(Reporter)

Comment 7

4 years ago
Has failed several times (until now 4 times) today, up from about once per week, bumping to P2
Priority: P3 → P2
Firefox 26 Beta is affected.
status-firefox26: ? → affected
Andrei and Cosmin, no further comments here regarding priorities? I would have expected that this gets a P1 given that it causes invalid results for beta builds and causes QA to manually verify this.
Flags: needinfo?(cosmin.malutan)
Flags: needinfo?(andrei.eftimie)
(Reporter)

Comment 10

4 years ago
Well let us bump the priority then.

Latest ondemand run this failed twice:
26.0, ru, 20131104182142, Windows 7
http://mozmill-release.blargon7.com/#/functional/report/d17690a112360a2b3155acaa270e7555
26.0, pl, 20131104182142, OSX 10.8.5
http://mozmill-release.blargon7.com/#/functional/report/d17690a112360a2b3155acaa2702b681

Running on both OS'es the test to see if we're able to reproduce the failure.

On daily we had this failure exactly once:
ESR 24.0.1, en-US, 20131011000503, OSX 10.8.5
http://mozmill-daily.blargon7.com/#/functional/report/6d6f6a58b02eeffc06eafa8beae120af

What we know:
- very low reproducibility rate,
- all OS'es, all machines affected.

The only failure we have on daily is on en-US, all others (13 in total) are on various locales.
status-firefox28: --- → ?
Flags: needinfo?(andrei.eftimie)
Priority: P2 → P1
I made testruns and I run the folder separately for 20 times and did't reproduced it.
status-firefox28: ? → ---
Flags: needinfo?(cosmin.malutan)
Priority: P1 → P2
Submit conflict
status-firefox28: --- → ?
Priority: P2 → P1
(Reporter)

Comment 13

4 years ago
I managed to reproduce the failure once in 400 runs with the following build:
26.0, ru, 20131104182142

Lets add some dumps and see if we get some relevant informations.
(Reporter)

Comment 14

4 years ago
More info.
When it fails, origNumCookies is 2, here:
http://hg.mozilla.org/qa/mozmill-tests/file/8afbeba6b6f4/tests/functional/testPreferences/testRemoveCookie.js#l81

Which might imply not that we don't remove the cookie, but we somehow have 2 cookies, and we remove one.

Reference

Pass:
> == origNumCookies: 1
> == Services.cookies.cookieExists: false
> == cookieRemoved: true

Fail:
> == origNumCookies: 2
> == Services.cookies.cookieExists: true
> == cookieRemoved: false

I'll further test this hypothesis.
Assignee: nobody → andrei.eftimie
(Reporter)

Comment 15

4 years ago
Update.

We have no cleanup problems.
The affected cookie is not present in setupModule() before nor after we call Services.cookies.removeAll();

The cookie appears to be added twice when we visit the page (?)
We then remove 1 instance, then fail when checking for its presence (because the "twin" cookie still exists).

In teardownModule() we still see the twin cookie, and we remove it with Services.cookies.removeAll();

===

I will hook up some code to check with API right before and after we visit the page to check the presence of the cookies.

From what I remember we are not able to set 2 cookies with the same name _unless_ they have different paths. I'll try gathering more info on the cookies to see how they differ, and maybe find out how they are stored twice.

This is going slower then anticipated, I only managed to fail the test once today, from 900 runs.
Has failed today with Beta it locale, on OS X 10.7.5:
http://mozmill-release.blargon7.com/#/functional/report/21341f02f219acb032f628de853ff14e
Can anyone please check the ratio between failures of beta/release vs. nightly builds? I can see nearly no failures for our nightlies.
No failures with nightly, I see only 2 with 24esrpre:
http://mozmill-daily.blargon7.com/#/functional/failure?branch=All&platform=All&from=2013-07-30&to=&test=%2FtestPreferences%2FtestRemoveCookie.js&func=testRemoveCookie.js%3A%3AtestRemoveCookie
(Reporter)

Comment 19

4 years ago
Note that this appears to be more common on exotic locales (I'm not sure of this, but that's what we've seen from the _few_ failures we have recorded). And we have no exotic locales on Nightly, and we only really run them ATM on Beta and Release.
Do you have overall numbers for us sorted by such locales on beta and release?
I don't think it's related to exotic locales, the link in the URL field, sorted by locales show en-US, de, it mostly. 
On Aurora we fail as well, with the 4 locales that we test.
(Reporter)

Comment 22

4 years ago
Looking more over the failures, versions, locales, I can't draw any real conclusion.
Locale probably has no influence for this failure.


Nr. of failures / locale
========================

Daily
-----
2 : en-US

Release
-------
3 : it
2 : es-ES
2 : en-US
2 : ru
2 : de
1 : sr
1 : pt-BR
1 : pl
1 : ml
1 : fr
1 : en-GB
1 : cs

Total
=====
20 failures recorded
Failed today with Esr24, on OS X 10.7:
http://mozmill-daily.blargon7.com/#/functional/report/8ff37f5518a39f6af2983cfe442c847b
Last time it failed on 27/12, decreasing the priority a little.
Priority: P1 → P2
It's been about a month. What's the status of this failure?
Flags: needinfo?(andrei.eftimie)
(Reporter)

Comment 26

4 years ago
This is still failing, but has a low occurrence rate,
I am able to reproduce this about 1 time in 900 runs.

I detailed what is happening in comment 15.
My next step (which I started, but didn't yet got around to finish it due to the low occurrence rate) was to try holding the browser open when this occurs to manually check _why_ we have 2 cookies saved instead of one.

With this I should be able to verify if this is a bug in Firefox or a problem in our tests.
Flags: needinfo?(andrei.eftimie)
Lets put it back into the general bucket, so that someone else could pick it up until we have time for. Andrei, would you like to mentor this bug?
Assignee: andrei.eftimie → nobody
Status: REOPENED → NEW
Priority: P2 → P3
(Reporter)

Comment 28

4 years ago
I would still like to finish what I started. But if anyone wants to help out that would be great.

Basically I have the specific test patched to sleep for a long time when this fails.
I only need to leave my VM run this for a day (last time I tried it didn't fail at all for a whole day).

But I need to check this at regular intervals to catch the failure.

When I do we should see if the 2 cookies are _identical_ if that is the case we might have a bug in Firefox. This is probably unlikely, since you can not have 2 cookies with the same name on the same domain + path.

If they are not identical, it might be that their path differs. In this case we might be able to understand what is going wrong.

==
For both cases this is likely either a asynchronicity issue or a race condition.
(Reporter)

Updated

4 years ago
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mentor=andrei_eftimie]
Would you mind to upload your patch for the test so others could directly use it?

To get started please have a look at https://developer.mozilla.org/en-US/docs/Mozmill_Tests
Whiteboard: [mozmill-test-failure][mentor=andrei_eftimie] → [mozmill-test-failure][mentor=andrei_eftimie][lang=js]
Failed today with OS X 10.8.5, beta:
http://mozmill-release.blargon7.com/#/functional/report/a438ea29b921b2e8124749eda90eeb8b
status-firefox28: ? → affected
(Reporter)

Comment 31

4 years ago
Failed again:
http://mozmill-release.blargon7.com/#/functional/report/4e9e123ff3e2bbcb949b2192928ad60b

I should run the test I had mentioned in comment 28 to see if we get somewhere.

Updated

4 years ago
Mentor: andrei.eftimie@softvision.ro
Whiteboard: [mozmill-test-failure][mentor=andrei_eftimie][lang=js] → [mozmill-test-failure][lang=js]
This failed today on Firefox 31.0 te on mm-osx-107-2 (2014-07-17_12-39-21)  : http://mozmill-release.blargon7.com/#/functional/report/db75145408bff5ccf8a072085b4db7ba
(Reporter)

Comment 33

3 years ago
This is still failing, though rarely, failed 4 times in the last month:
http://mozmill-release.blargon7.com/#/functional/failure?branch=All&platform=All&from=2013-07-30&test=%2FtestPreferences%2FtestRemoveCookie.js&func=testRemoveCookie.js%3A%3AtestRemoveCookie
status-firefox25: affected → wontfix
status-firefox26: affected → wontfix
status-firefox27: ? → wontfix
status-firefox28: affected → wontfix
status-firefox31: --- → affected
status-firefox32: --- → affected
status-firefox-esr24: affected → wontfix
(In reply to Andrei Eftimie from comment #33)
> This is still failing, though rarely, failed 4 times in the last month:
> http://mozmill-release.blargon7.com/#/functional/
> failure?branch=All&platform=All&from=2013-07-
> 30&test=%2FtestPreferences%2FtestRemoveCookie.js&func=testRemoveCookie.
> js%3A%3AtestRemoveCookie

Wrong link? I cannot find a single failure for that since December last year.
status-firefox25: wontfix → ---
status-firefox26: wontfix → ---
status-firefox27: wontfix → ---
status-firefox28: wontfix → ---
status-firefox33: --- → ?
status-firefox34: --- → ?
status-firefox-esr17: wontfix → ---
status-firefox-esr31: --- → affected
(Reporter)

Comment 35

3 years ago
Indeed, that was the wrong link.
Corrected: http://mozmill-release.blargon7.com/#/functional/failure?app=Firefox&branch=All&platform=All&from=2014-07-01&test=%2FtestPreferences%2FtestRemoveCookie.js&func=testRemoveCookie
Failed with release bg, on Win Vista:
http://mozmill-release.blargon7.com/#/functional/report/eefb0f8a26a2a6196c62c274481ed97d
(Assignee)

Updated

3 years ago
Assignee: nobody → teodor.druta
(Assignee)

Comment 37

3 years ago
Created attachment 8506895 [details] [diff] [review]
b856084.patch

The issue is related to the filter field functionality not displaying query results fast enough resulting in two cookies in the cookies list, so the test actually removes the incorrect cookie and fails when checking if the cookie has been removed.
This patch should fix this issue.
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Attachment #8506895 - Flags: review?(andrei.eftimie)
Attachment #8506895 - Flags: review?(andreea.matei)
(Reporter)

Comment 38

3 years ago
Comment on attachment 8506895 [details] [diff] [review]
b856084.patch

Review of attachment 8506895 [details] [diff] [review]:
-----------------------------------------------------------------

Interesting train of thought, but I can't follow it.

I've tested numerous times, and I don't see how we could have 2 cookies shown. There can only be one.
The opposite also holds. If we were to try removing the cookie _before_ it was shown, we would immediately fail when checking the removed cookie and the rowCount:
>  expect.ok(cookieRemoved, "The cookie has been removed");
>  expect.equal(cookiesList.view.rowCount, (origNumCookies - 1),
>               "There is one less cookie than before");

Please add more data here.

Are you able to reproduce the issue?
Having 2 different cookies and the test removing the wrong one might explain the failure, but I don't see from where the second cookie might come from...

::: firefox/tests/functional/testPreferences/testRemoveCookie.js
@@ +92,5 @@
> +
> +  // wait for the slow search cookies functionality in firefox
> +  assert.waitFor(() => (cookiesList.view.rowCount === 1), 
> +                 "There is only one row in the cookies list !");
> +  

nit: make sure you don't have trailing whitespaces
Attachment #8506895 - Flags: review?(andrei.eftimie)
Attachment #8506895 - Flags: review?(andreea.matei)
Attachment #8506895 - Flags: review-
If you see more than 1 cookie please dump all the cookie information to the console. This might help to figure out what's wrong. If that is really the case it should be a bug in Firefox and no workaround is warranted.
(Reporter)

Comment 40

3 years ago
Actually your patch might indeed fix the issue.(In reply to Andrei Eftimie from comment #38)
> The opposite also holds. If we were to try removing the cookie _before_ it
> was shown, we would immediately fail when checking the removed cookie and
> the rowCount:
> >  expect.ok(cookieRemoved, "The cookie has been removed");
> >  expect.equal(cookiesList.view.rowCount, (origNumCookies - 1),
> >               "There is one less cookie than before");

Actually this is exactly how the test is failing. So indeed we might click on "Remove Cookie" before the list is populated, thus failing to remove it...

In this case your patch should solve the issue.
(Reporter)

Comment 41

3 years ago
Comment on attachment 8506895 [details] [diff] [review]
b856084.patch

Review of attachment 8506895 [details] [diff] [review]:
-----------------------------------------------------------------

Fix the mentioned nits, and add a commit message please.

::: firefox/tests/functional/testPreferences/testRemoveCookie.js
@@ +89,5 @@
>  
>    // Get the number of cookies in the file manager before removing a single cookie
>    var cookiesList = aController.window.document.getElementById("cookiesList");
> +
> +  // wait for the slow search cookies functionality in firefox

Rephrase into: "Wait for the cookie list to load"

@@ +91,5 @@
>    var cookiesList = aController.window.document.getElementById("cookiesList");
> +
> +  // wait for the slow search cookies functionality in firefox
> +  assert.waitFor(() => (cookiesList.view.rowCount === 1), 
> +                 "There is only one row in the cookies list !");

"There is one item in the cookie list."
(Reporter)

Updated

3 years ago
status-firefox31: affected → wontfix
status-firefox32: affected → wontfix
status-firefox33: ? → affected
status-firefox34: ? → affected
status-firefox35: --- → affected
status-firefox36: --- → affected
(Assignee)

Comment 42

3 years ago
Created attachment 8508573 [details] [diff] [review]
b856084v2.patch

Updated the patch following the review notes.
Attachment #8506895 - Attachment is obsolete: true
Attachment #8508573 - Flags: review?(andrei.eftimie)
Attachment #8508573 - Flags: review?(andreea.matei)
Comment on attachment 8508573 [details] [diff] [review]
b856084v2.patch

Review of attachment 8508573 [details] [diff] [review]:
-----------------------------------------------------------------

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/b477768a6fdf (default)

Please check backporting, if the patch applies cleanly and tests well. Thanks!
Attachment #8508573 - Flags: review?(andrei.eftimie)
Attachment #8508573 - Flags: review?(andreea.matei)
Attachment #8508573 - Flags: review+
status-firefox36: affected → fixed
(Assignee)

Comment 44

3 years ago
Created attachment 8514154 [details] [diff] [review]
b856084v2ESR31ESR24.patch

The patch applies cleanly on all branches except mozilla-esr31 and mozilla-esr24.
I have attached a patch for this branches.
Attachment #8514154 - Flags: review?(andrei.eftimie)
Attachment #8514154 - Flags: review?(andreea.matei)
(Reporter)

Comment 45

3 years ago
Lets get this transplanted, hopefully no more failures!
This is a point fix in the test itself, I will land across branches:
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/2e55e5da4350 (mozilla-aurora)
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/d2aff51c188b (mozilla-beta)
remote:   https://hg.mozilla.org/qa/mozmill-tests/rev/9ff4d8332d7e (mozilla-release)
status-firefox33: affected → fixed
status-firefox34: affected → fixed
status-firefox35: affected → fixed
(Reporter)

Comment 46

3 years ago
Comment on attachment 8514154 [details] [diff] [review]
b856084v2ESR31ESR24.patch

Review of attachment 8514154 [details] [diff] [review]:
-----------------------------------------------------------------

https://hg.mozilla.org/qa/mozmill-tests/rev/333c8d5f61eb (mozilla-esr31)
Attachment #8514154 - Flags: review?(andrei.eftimie)
Attachment #8514154 - Flags: review?(andreea.matei)
Attachment #8514154 - Flags: review+
Attachment #8514154 - Flags: checkin+
(Reporter)

Comment 47

3 years ago
Thanks Teodor for figuring this out and providing a fix.
Lets hope this is the last we'll see of this failure.

Please reopen if this doesn't fully solve the issue.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago3 years ago
status-firefox-esr31: affected → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.