491898 - Random Test Failure (browser_passwordmgrdlg.js)

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Shawn Wilsher :sdwilsh]

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'secret' - Got 0, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | Timed out
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'secret' - Got 0, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | Timed out

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1241712330.1241719484.1806.gz&fulltext=1
OS X 10.5.2 mozilla-central unit test on 2009/05/07 09:05:30

Comment 1

[Justin Dolske [:Dolske]]

The cycle right before this was testing the same changeset, and it was green. So something somewhere is clearly random. :(

Comment 2

[(no longer active)]

The details of the test run, for future reference:

TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | Filter box should initially be empty
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | There should be 10 passwords initially
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'pass'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match ''
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'moz'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozi'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozil'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozill'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozilla'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'mozilla.com'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'user '
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match ' user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match 'http'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'https'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'secret'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'secret!'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'pass'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match ''
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'moz'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozi'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozil'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozill'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozilla'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'mozilla.com'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'user '
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match ' user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match 'http'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'https'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'secret'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'secret!'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'pass'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'pass'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match ''
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match ''
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'moz'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'moz'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozi'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozi'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozil'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozil'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozill'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozill'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozilla'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 7 logins should match 'mozilla'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'mozilla.com'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'mozilla.com'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 4 logins should match 'user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'user '
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'user '
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match ' user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match ' user'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match 'http'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 10 logins should match 'http'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'https'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 1 logins should match 'https'
TEST-PASS | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 0 logins should match 'secret'
NEXT ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | 2 logins should match 'secret' - Got 0, expected 2
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/passwordmgr/test/browser/browser_passwordmgrdlg.js | Timed out

Comment 3

[(no longer active)]

Attachment: Patch (v1)

The problem here is that toggleShowPasswords uses the closing of the confirmation prompt as a sign of the completion of the filtering of passwords after the toggling completes, and tries to compensate with a setTimeout call, but it turns out that this is not enough.

This patch explicitly adds a notification after the whole operation of toggling passwords completes and uses that notification inside the test to figure out if it's time to run the callback function inside of toggleShowPasswords.

Comment 4

[Justin Dolske [:Dolske]]

Comment on attachment 376607 [details] [diff] [review]
Patch (v1)

Meh. I'm not really thrilled about adding a useless-except-for-tests notification, but this is probably easiest. The only alternative I can think of would be to have the test poll the state of the Show/Hide button every X ms until it changes, which is also kind of meh.

Comment 5

[Justin Dolske [:Dolske]]

...despite my grumpyness at the code, thanks for looking into this so quickly!

Comment 6

[(no longer active)]

(In reply to comment #4)
> (From update of attachment 376607 [details] [diff] [review])
> Meh. I'm not really thrilled about adding a useless-except-for-tests
> notification, but this is probably easiest. The only alternative I can think of
> would be to have the test poll the state of the Show/Hide button every X ms
> until it changes, which is also kind of meh.

I'm not thrilled about it either, but on the bright side, a notification with no listeners attached should be fairly cheap, and this is not the first place in the code where such test-only notifications are used.  :-)

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/83683c0185c8

Comment 8

[(no longer active)]

Comment on attachment 376607 [details] [diff] [review]
Patch (v1)

This would be a very nice fix for 1.9.1 and is virtually no-risk.  It will prevent possible random oranges on the test landed in bug 451352.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242233337.1242237658.4557.gz
Linux mozilla-1.9.1 unit test on 2009/05/13 09:48:57  

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242226137.1242231946.28097.gz
Linux mozilla-1.9.1 unit test on 2009/05/13 07:48:57

Comment 10

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1242463896.1242469513.16124.gz
Linux comm-1.9.1 unit test on 2009/05/16 01:51:36

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 376607 [details] [diff] [review]
Patch (v1)

a191=beltzner

This doesn't thrill me either, but for the good of development ...

Comment 12

[(no longer active)]

Mike, you forgot to toggle the flag on the patch itself...

I'm going to land this anyway...

Comment 13

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7c2ee6fff7c3