Closed Bug 948782 Opened 6 years ago Closed 6 years ago

make test_alerts_noobserve.html wait until its alerts have disappeared before finishing

Categories

(Toolkit :: General, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29
Tracking Status
firefox28 --- fixed
firefox29 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 3 obsolete files)

Attached file patch (obsolete) —
I've had some issues with test_multiple_alerts.html failing with this error (on try runs, with my patch queue applied):

157682 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/alerts/test/test_multiple_alerts.html | Second alert should be opened in the same position. - got 32, expected 33

After adding some debugging, it looks like the previous test, test_alerts_noobserve.html, leaves notification visible when it finishes and advances to the next test, and that the previous notification can cause the position of one of the notifications from test_multiple_alerts.html to be placed differently.  I think it would be best if test_alerts_noobserve.html waited at the end until the notification disappears naturally (still without adding an observer, which is what the test is testing).
Attachment #8345669 - Flags: review?(wchen)
Blocks: 773296
Comment on attachment 8345669 [details]
patch

More work required it seems.
Attachment #8345669 - Flags: review?(wchen)
Attached patch patch v2 (obsolete) — Splinter Review
This seemed to work better, also adding in some waiting for notifications to disappear in test_multiple_alerts.html.

https://tbpl.mozilla.org/?tree=Try&rev=cc71827ac1d5
Attachment #8345669 - Attachment is obsolete: true
Attachment #8345725 - Flags: review?(wchen)
Attachment #8345725 - Attachment is patch: true
Attached patch patch v2.1 (obsolete) — Splinter Review
Sorry one final tweak.
Attachment #8345725 - Attachment is obsolete: true
Attachment #8345725 - Flags: review?(wchen)
Attachment #8346185 - Flags: review?(wchen)
Attached patch patch v3Splinter Review
From IRC conversations, the theory is that test_alerts_noobserve.html might have its alert start showing only after that test has finished.  So in this patch we wait a bit at the end of the test, before checking whether the alert has disappeared.  This make it no longer necessary to wait for alerts to disappear in test_multiple_alerts.html, though I've included a test that there aren't any, at the beginning of the test.

https://tbpl.mozilla.org/?tree=Try&rev=c1e42e85d12c
Attachment #8346185 - Attachment is obsolete: true
Attachment #8346185 - Flags: review?(wchen)
Attachment #8346287 - Flags: review?(wchen)
Comment on attachment 8346287 [details] [diff] [review]
patch v3

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

test_alerts_noobserve.html certainly wasn't cleaning up after itself before. The logs show that it is now waiting for the alert window to close before finishing.
Attachment #8346287 - Flags: review?(wchen) → review+
https://hg.mozilla.org/mozilla-central/rev/8344fa6ecea6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.