976182 - ICE connection Mochitest can result in test timeout

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Nils Ohlmeier [:drno]]

The new ICE connection tests in media/dom/tests/mochitest can block further test execution if the initial connection state is in "failed" already.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: ice_connection_state_failed.patch

Comment 2

[Nils Ohlmeier [:drno]]

https://tbpl.mozilla.org/?tree=Try&rev=20e75f203d37

Comment 3

[Byron Campen [:bwc]]

Comment on attachment 8380786 [details] [diff] [review]
ice_connection_state_failed.patch

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

This will do the job, although it will break if we start returning stuff like "disconnected" or "closed" someday. We could bail if we aren't in either "new" or "checking" instead. Or we could leave a note or something. Up to you.

::: dom/media/tests/mochitest/pc.js
@@ +1467,5 @@
>  
>    /**
> +   * Returns if the ICE the connection state is "failed".
> +   *
> +   * @returns {boolean} True is the connection state is "failed", otherwise false.

Minor typo, s/is/if/. Seems to show up other places.

Comment 4

[Nils Ohlmeier [:drno]]

Attachment: ice_connection_state_failed.patch

This patch should address bwc's concerns about the ICE states as well as the NITs in the comments.

Comment 5

[Nils Ohlmeier [:drno]]

https://tbpl.mozilla.org/?tree=Try&rev=06ec8f674547

Comment 6

[Byron Campen [:bwc]]

Comment on attachment 8385024 [details] [diff] [review]
ice_connection_state_failed.patch

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

Looks good to me.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4246950b826f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4246950b826f