1143937 - Make BrowserTestUtils.waitForEvent not use flaky timeouts and cover more use cases

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[:Paolo Amadini]

Similar change to bug 1142108, also removing the custom timeout since the test harness should take care of this for us.

Comment 1

[:Paolo Amadini]

Attachment: The patch

Comment 2

[Steven MacLeod [:smacleod]]

Comment on attachment 8578342 [details] [diff] [review]
The patch

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

Overall looks good, just the one problem with checkFn.

::: testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ +115,5 @@
> +            if (checkFn && !checkFn(event)) {
> +              return;
> +            }
> +          } finally {
> +            subject.removeEventListener(eventName, listener);

This will run even when the "return;" is hit, so if "checkFn" gives false we'll remove the listener and never hit the event.

Comment 3

[:Paolo Amadini]

Attachment: Updated patch

It seems I made the same mistake in TestUtils.topicObserved.

Comment 4

[Steven MacLeod [:smacleod]]

Comment on attachment 8579597 [details] [diff] [review]
Updated patch

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

LGTM

Comment 5

[:Paolo Amadini]

https://hg.mozilla.org/integration/fx-team/rev/9d01ed5c9fc5

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9d01ed5c9fc5