843553 - Test failure "Checkbox ID: acceptCookies could not be checked/unchecked" in /testPreferences/testDisableCookies.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Daniela Petrovici]

Happened today on MAC 10.7.5 and FF Nightly:
http://mozmill-ci.blargon7.com/#/functional/report/f36358d058daf73ddbf781501671301d

Happens because of this line: http://hg.mozilla.org/qa/mozmill-tests/file/0dd2dc62cc0e/tests/functional/testPreferences/testDisableCookies.js#l67

It might be that the 0.5 seconds sleep is not enough for waiting before clicking on the checkbox

Comment 1

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

Ouch, so there is still a silly sleep() call. Yes, we should get this eliminated.

Comment 2

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

We might want to check if other cookie tests have the same sleep call included.

Comment 3

[Daniela Petrovici]

Attachment: patch v1.0

I have removed the sleep calls from the testPreferences/ cookies tests and added an assert.waitFor. 

The issue reproduces constantly under heavy load on the local MAC machine I have (e.g. A failing run: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4c23eec5). The problem is not that we cannot focus on the elements after changing history mode as the comments in the tests suggest, but that we are not waiting for the history mode to be fully changed before continuing the test. 

The same issue applies for bug 799032, bug 833805 and bug 813988 - we are not waiting when changing history mode, and adding the wait for history mode fixes those issues, too.

Also, there were two other sleep calls in the following files:
http://hg.mozilla.org/qa/mozmill-tests/file/f1f141b9ff87/tests/functional/testPreferences/testDisableCookies.js#l85
http://hg.mozilla.org/qa/mozmill-tests/file/f1f141b9ff87/tests/functional/testPreferences/testEnableCookies.js#l99
http://hg.mozilla.org/qa/mozmill-tests/file/f1f141b9ff87/tests/functional/testPreferences/testRemoveCookie.js#l79

These sleep calls are not needed in case we are properly waiting for the history mode to be changed.

Passing run for this patch: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4c2b5f06

Comment 4

[Daniela Petrovici]

Since we replace the sleep calls here for all the cookies tests and this fixes bugs: 799032, 833805 and 813988, I am marking this bug as blocking the others

Comment 5

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

Comment on attachment 719044 [details] [diff] [review]
patch v1.0

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

::: tests/functional/testPreferences/testDisableCookies.js
@@ +59,5 @@
>    controller.waitForElement(historyMode, TIMEOUT);
>    controller.select(historyMode, null, null, "custom");
> +  assert.waitFor(function () {
> +    return historyMode.getNode().value === "custom";
> +  }, "History mode is set to custom");

That sounds like a bug in Mozmill. Don't we wait until the apropriate entry has been selected? We should file a Mozmill bug if that also happens in Mozmill 2.

Comment 6

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 719044 [details] [diff] [review]
patch v1.0

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

Looks good, landed for default:
http://hg.mozilla.org/qa/mozmill-tests/rev/0cfefc928c33 (default)

This test was affected only on default, but giving the change on the others as well, I say we backport to all branches. 
Lets wait for some reports first.

Comment 7

[Daniela Petrovici]

(In reply to Henrik Skupin (:whimboo) from comment #5)
> Comment on attachment 719044 [details] [diff] [review]
> patch v1.0
> 
> Review of attachment 719044 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: tests/functional/testPreferences/testDisableCookies.js
> @@ +59,5 @@
> >    controller.waitForElement(historyMode, TIMEOUT);
> >    controller.select(historyMode, null, null, "custom");
> > +  assert.waitFor(function () {
> > +    return historyMode.getNode().value === "custom";
> > +  }, "History mode is set to custom");
> 
> That sounds like a bug in Mozmill. Don't we wait until the apropriate entry
> has been selected? We should file a Mozmill bug if that also happens in
> Mozmill 2.

Mozmill 1.5 - The controller.select does not have a waitFor call anywhere. It is just open, click and close. So we are not waiting for the appropiate entry to be selected.

And it looks the same in Mozmill 2.0. I have logged bug 846277 for this issue.

Do we want to backout the patch from Default branch or leave it until a fix is done in Mozmill?

Comment 8

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

We want to land this across all branches. We will not spin another Mozmill 1.5 release for this issue.

Comment 9

[Daniela Petrovici]

Attachment: patch v1.0 for all other branches

The patch for default does not apply cleanly on Aurora branch so I have created a patch for that. The patch for Aurora applies cleanly on Beta, Release and ESR17 branches.

Reports:
1) Aurora: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca3b1f5
http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca3ced4
2) Beta: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca44954
http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca46273
3) Release: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca4b825
http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca4917e
4) ESR17: http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca61501
http://mozmill-crowd.blargon7.com/#/functional/report/66aad0ebfa7a01580628b3af4ca5d7d8

Comment 10

[Andreea Matei [:AndreeaMatei]]

Transplanted as:
http://hg.mozilla.org/qa/mozmill-tests/rev/e935ccdcf975 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/10a90a2f4f35 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/459ac637c168 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/5e5c323c5daf (esr17)

Thanks Daniela!