841355 - Test failure "Password notification should be closed" in some functional tests that use notification.getNode().state

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Daniela Petrovici]

Started happening today in two of the tests that use notification.getNode().state:
Failing test: /testPreferences/testPasswordSavedAndDeleted.js on Latest Nightly, MAC OS 10.8.2 en-US:
http://mozmill-ci.blargon7.com/#/functional/report/a83c700664548dba07298b74bf58706f
Linux x64 and Nightly:
http://mozmill-ci.blargon7.com/#/functional/report/a83c700664548dba07298b74bf5a882b
Linux x86 and Nightly:
http://mozmill-ci.blargon7.com/#/functional/report/a83c700664548dba07298b74bf5a82fb

Failing test: /restartTests/testPreferences_masterPassword/test1.js
in Latest Nightly, MAC OS 10.8.2:
http://mozmill-ci.blargon7.com/#/functional/report/a83c700664548dba07298b74bf58bfab

I was able to reproduce this locally on MAC OS 10.7.5 with both tests and the latest Nightly build:
http://mozmill-crowd.blargon7.com/#/functional/report/a83c700664548dba07298b74bf5bb90c
http://mozmill-crowd.blargon7.com/#/functional/report/a83c700664548dba07298b74bf5c0f4

The issue does not reproduce with yesterday build.

The pushlog is here: http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=aceeea086ccb and I will try to find the change that caused this.

Comment 1

[Daniela Petrovici]

http://mozmill-ci.blargon7.com/#/functional/failure?branch=All&platform=All&from=2013-02-14&to=&test=%2FtestPreferences%2FtestPasswordSavedAndDeleted.js&func=testPasswordSavedAndDeleted.js%3A%3AtestSaveAndDeletePassword
http://mozmill-ci.blargon7.com/#/functional/failure?branch=All&platform=All&from=2013-02-14&to=&test=%2FrestartTests%2FtestPreferences_masterPassword%2Ftest1.js&func=test1.js%3A%3AtestSetMasterPassword

Comment 2

[Daniela Petrovici]

http://mozmill-ci.blargon7.com/#/functional/report/a83c700664548dba07298b74bf5f7561

Comment 3

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

I can replicate this locally too. It appears if I add a one second sleep before /tests/functional/testPreferences/testPasswordSavedAndDeleted.js:54 the test passes. Continue looking for a regressing changeset, but we might need to bolster this with a better way of waiting for the button before we click it.

Comment 4

[Andrei Eftimie]

Attachment: skip patch

Prepared a skip patch if needed.

Comment 5

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

Comment on attachment 714340 [details] [diff] [review]
skip patch

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/93688907cd52

Unfortunately I did this before noticing the commit message. Please be more descriptive in future. In this case, something like "Bug 841355 - Skipped tests due to test failure: 'Password notification should be closed'"

Comment 6

[Daniela Petrovici]

Found the pushlog and changeset due to which it failed:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b3e65fe37681&tochange=d5dcdeae4bd2

http://hg.mozilla.org/mozilla-central/rev/b3e65fe37681 - passing
http://hg.mozilla.org/mozilla-central/rev/d5dcdeae4bd2 - failing

Comment 7

[Daniela Petrovici]

I think that it might be related to bug 583175 - merged changeset da6fdc53ba9c

Comment 8

[Matthew N. [:MattN]]

The fix is to disable the security delay as we do for some testsuites in m-c[1] by setting security.notification_enable_delay to 0.

[1] https://hg.mozilla.org/mozilla-central/diff/da6fdc53ba9c/build/automation.py.in

Comment 9

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

We could patch Mozmill for that but I wouldn't go and fire another release for 1.5 because of that change. So for our tests I would say lets set the preference to 0s until Mozmill can handle it.

Comment 10

[Daniela Petrovici]

Attachment: patch v1.0

I have re-enabled the tests and added preferences for all tests that are using the notification.getNode().state. This means all three tests were modified, not just the two that failed. 

This is because when changing just the testPreferences/testPasswordSavedAndDeleted.js and running through mozmill-automation the test is still failing every time:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016086f87

When changing the /testPasswordManager/testPasswordNotificationBar.js too, the tests are not failing anymore:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf78150160cf214
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf78150160d01f7

Comment 11

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

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

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

Beside my comment given inline please also file a bug for Mozmill.

::: tests/functional/restartTests/testPreferences_masterPassword/test1.js
@@ +13,4 @@
>  const LOCAL_TEST_FOLDER = collector.addHttpResource('../../../../data/');
>  const LOCAL_TEST_PAGE = LOCAL_TEST_FOLDER + 'password_manager/login_form.html';
>  
> +const SECURITY_NOTIFICATION_DELAY = "security.notification_enable_delay";

I want to see an XXX description here so we know when to remove the code again.

Comment 12

[Daniela Petrovici]

Attachment: patch v1.1

Logged bug 842472 for the Mozmill changes and updated patch based on review

Comment 13

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 715421 [details] [diff] [review]
patch v1.1

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

No need for this changes in our tests as we have discussed on IRC to make them in mozmill directly.

Comment 14

[Andreea Matei [:AndreeaMatei]]

Backed out on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/d4e049e431b6 (default)

Waiting confirmation that is all good to do the same on aurora as well.

Comment 15

[Daniela Petrovici]

I have ran tests on all platforms with Aurora:
Windows:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016929820
MAC:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf7815016925384
Linux:
http://mozmill-crowd.blargon7.com/#/functional/report/f36358d058daf73ddbf781501697b532

Comment 16

[Andreea Matei [:AndreeaMatei]]

Backed out from aurora:
http://hg.mozilla.org/qa/mozmill-tests/rev/c7d07fb5ff1d (aurora)

And I think we're done here :) Thanks!