Closed Bug 1382972 Opened 2 years ago Closed 2 years ago

Intermittent /webrtc/RTCPeerConnection-addIceCandidate.html | Add valid candidate should never resolve when pc is closed - Peer connection is closed

Categories

(Core :: WebRTC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: mjf)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell disabled])

Attachments

(1 file)

That is https://github.com/soareschen/web-platform-tests/blob/master/webrtc/RTCPeerConnection-addIceCandidate.html but I'm not sure how we injest wpt testing into our CI so I don't know which rev of that we ran, specifically.
if this was fixed upstream, we can ask :jgraham to sync upstream to our source.  :jgraham, how can we determine which revision of a wpt test is in our tree?
Flags: needinfo?(james)
Whiteboard: [stockwell needswork]
Quite frankly I think these WPT tests are just really poorly written. I highly recommend to disable/skip these failing tests for now and complain upstream.
:jgraham who is ni'd on this bug already would have a bigger picture about the origination of the tests and expectations/requirements of them prior to being in-tree.  He might have ideas.  If there are a bunch of these that are randomizing us, we either have a flaky browser (as other browser vendors run these same tests), flaky infrastructure, or possibly others are just skipping these tests and we are the only ones running them.
I created https://github.com/w3c/web-platform-tests/issues/6611 with a suggestion of how make these tests more reliable. I think we should skip them until they are fixed/improved to avoid the noise.
Looks like there is now some PR upstream to remove these tests: https://github.com/w3c/web-platform-tests/pull/6616. If someone marks it reviewed I can make sure it's merged and then start a new wpt sync.

jmaher: In general [vcs] log testing/web-platform/tests will tell you the last upstream revision that was imported e.g.

james@ginny:~/develop/gecko$ git log --grep=revision --oneline -n1 -- testing/web-platform/tests/
7f18a43b0677 Bug 1381842 - Update web-platform-tests to revision 85ae24e3dc80ee63b6dc2ed78a922cff68c6e819, a=testonly
Flags: needinfo?(james)
Rank: 20
Priority: -- → P2
Assignee: nobody → mfroman
For now I'm going to disable this test in our repo.
Comment on attachment 8890034 [details]
Bug 1382972 - disable wpt test RTCPeerConnection-addIceCandidate.html

https://reviewboard.mozilla.org/r/161088/#review166528

This is a rather extreme solution :-)

I was more aiming for skipping only the tests which are causing problems (or skip all which are written with the same problematical pattern).

I guess this is a way to skip them http://searchfox.org/mozilla-central/source/testing/web-platform/meta/webrtc/RTCPeerConnection-onnegotiationneeded.html.ini#17
Attachment #8890034 - Flags: review?(drno) → review-
:jgraham is this a way to skip a single test in WPT http://searchfox.org/mozilla-central/source/testing/web-platform/meta/webrtc/RTCPeerConnection-onnegotiationneeded.html.ini#17 ?

Or is there a better way to skip/not execute a single test within a given file?
Flags: needinfo?(standard8) → needinfo?(james)
[RTCPeerConnection-onnegotiationneeded.html]
  type: testharness
  expected: TIMEOUT
  [task for negotiationneeded event should be enqueued for next tick]
    expected: TIMEOUT

  [addTransceiver() should fire negotiationneeded event]
    expected: NOTRUN

  [Calling addTransceiver() twice should fire negotiationneeded event once]
    expected: FAIL

  [negotiationneeded event should not fire if signaling state is not stable]
    expected: FAIL

  [negotiationneeded event should fire only after signaling state go back to stable]
    disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1382972
    expected: NOTRUN

Will cause the result of "negotiationneeded event should fire only after signaling state go back to stable" to be ignored. The test will still run of course.
Flags: needinfo?(james)
Comment on attachment 8890034 [details]
Bug 1382972 - disable wpt test RTCPeerConnection-addIceCandidate.html

https://reviewboard.mozilla.org/r/161088/#review166910

LGTM
Attachment #8890034 - Flags: review?(na-g) → review+
Pushed by mfroman@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/78c932ff7deb
disable wpt test RTCPeerConnection-addIceCandidate.html r=ng
https://hg.mozilla.org/mozilla-central/rev/78c932ff7deb
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Whiteboard: [stockwell needswork] → [stockwell disabled]
You need to log in before you can comment on or make changes to this bug.