Open Bug 660585 Opened 12 years ago Updated 8 months ago

test_prompt.html has various intermittent failures

Categories

(Toolkit :: Password Manager, defect, P3)

Product:
Toolkit
Component:
Password Manager
Platform:
All
Linux
Type:
defect

Tracking

()

People

(Reporter: jdm, Unassigned)

References

Details

(Keywords: intermittent-failure, Whiteboard: [disabled on linux] [passwords:tech-debt]) 

I don't think the orange spam bugs are a great place to be doing analysis, so I'm going to use this bug instead.  I did some investigation, and came up with a theory for at least one problem that is causing oranges.

Looking at http://tinderbox.mozilla.org/showlog.cgi?log=Try/1306639585.1306639986.25461.gz I can see the problem where the notification that we get in clearIt is not the one we're expecting from 1003, but instead the notification from 506. Notice that 506 claims it succeeds, because we always set the authInfo that's being checked - we don't actually check the login that the login manager stores.  Test 507's notification is wrong, because it just shows us updating the last used time of the login, when we should be resetting from the new value to the old one.  The only reason this could be occurring is that the password change in test 506 is actually triggering a popup as well as a dialog - that's why we don't see a notification until test 1003, which is the first one that attempts to handle popups. The only bit that might not add up with this analysis is the line 

>5141 INFO TEST-PASS | /tests/toolkit/components/passwordmgr/test/test_prompt.html | got popup notification
>5142 INFO TEST-PASS | /tests/toolkit/components/passwordmgr/test/test_prompt.html | Looking for action at index 0
>5143 INFO TEST-PASS | /tests/toolkit/components/passwordmgr/test/test_prompt.html | at least one notification displayed
>5144 INFO TEST-PASS | /tests/toolkit/components/passwordmgr/test/test_prompt.html | 1 notifications

which seems to say that there is only one password popup in 1003, not two like I would expect. However, I'm not certain precisely what it's counting, so it may not mean anything.
(In reply to comment #0)
> I don't think the orange spam bugs are a great place to be doing analysis,
> so I'm going to use this bug instead.

Yes, thank you.


> Notice that 506 claims it succeeds, because we always set the authInfo
> that's being checked - we don't actually check the login that the login
> manager stores.  Test 507's notification is wrong, because it just shows us
> updating the last used time of the login, when we should be resetting from
> the new value to the old one.

I'm not sure I'm following.

Are you talking about output generated by the observer? ("observer for passwordmgr-storage-changed / modifyLogin") And how the newLogin and oldLogin dumps appear to be identical?

That is odd.

I'd expect there to be a notification between the output of "handleDialog running for test 506" and "handleDialog running for test 507", since the password is supposed to be changing. But we don't get a notification until after 507's "handleDialog done".

Oddly the same thing is happening when the test is green! Could be a different issue, then, but lemme understand this first so that we know we're in a good state for the rest of the test.
Oooooh. The stored login's password isn't changing because the test doesn't actually try to click the "change password" button in the notification (or, really, even look for a notification at all). That's confusing, I probably should have put a comment there!

I think the assumption was that we could just ignore the notification bars, since the main goal was to ensure that the prompt APIs returned the right things (and not so much to see that pwmgr was doing the right thing while prompts were going on).

I wonder if some explicit test logging of what notifications are present would help. Maybe we should wait for (and clear) the notifications. Though I'm not sure that will help with the test failures at hand...
Pushed http://hg.mozilla.org/mozilla-central/rev/02d683d66b97 to get some more info for debugging.
Got a failure with the logging: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1307240220.1307240486.31833.gz

Seems that somehow the change-password notification from test 506 is what's getting clicked in 1003's handleLoad(). But that shouldn't be happening because test 1003's handleDialog() is changing the password for a different login (and it's change-password notification should replace the one from 506).

So either PopupNotifications sometimes isn't replacing the old notification, or it does so too late (ie, race). The only other difference I noticed was the failure log has "is popup panel open? true" for 1004's handleLoad(). Not sure if that's significant... Need to ponder that (does it imply anything about when the panel is shown vs when the http auth prompt is shown? does the prompt dismiss the panel?).
OK, I've resisted saying this for a long time, but I think by this point this test is hurting us more than it benefits us.  Justin, how do you feel about me disabling it on mozilla-central until this bug is resolved?  I think by now the issues are well understood, and any trial to get to a fix can be done on the try server with a patch to re-enable the test...

/me is sad :(

This test has been somewhat rewritten years ago so this may be fixed but I didn't attempt a try push on Linux with it enabled.

Priority: -- → P3
Hardware: x86 → All
Whiteboard: [disabled on linux] → [disabled on linux] [passwords:tech-debt]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.