Currently, signaling_unittests is the only test case we have for integration testing without needing a display (as opposed to the mochitests). Unfortunately, it takes a very long time to run (longer than the mochitest suite), is full of crufty copy/paste code, and has many test-cases that belong in jsep_session_unittest. Additionally, the long run time means that this test is not run in CI, which leads to frequent bustage. Some time needs to be spent fixing these problems.

Not useful for CI, but useful for dev testing using gtest-parallel.

Moving the CI-related stuff over to bug 1140584, since I want to land this stuff sooner rather than later.

/r/4999 - Bug 1136252 - Part 1: Increase the number of instances of signaling_unittests that can be run in parallel. /r/5001 - Bug 1136252 - Part 2: Wait for less RTP in signaling_unittests. /r/5003 - Bug 1136252 - Part 3: Remove some unnecessary sleeps in signaling_unittests. /r/5005 - Bug 1136252 - Part 4: Remove/consolidate code in signaling_unittests. Includes removing most SDP checks, since that belongs in jsep_session_unittest. /r/5007 - Bug 1136252 - Part 5: Fix bug where candidates could be trickled before setRemote during renegotiation. /r/5009 - Bug 1136252 - Part 6: Extend timeouts for RTP/RTCP, until bug 1137948 can be fixed. Pull down these commits: hg pull review -r 1d97b8d410b2aeed92db1fc9223db57f3a09683a

Comment on attachment 8574191 [details] MozReview Request: bz://1136252/bwc https://reviewboard.mozilla.org/r/4997/#review3999 ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > // TODO(bug 1056650): once we have multistream support, all we need to do > // here is run a test with two audio streams, since that will prevent the > // PTs from being unique Want to check this comment?

https://reviewboard.mozilla.org/r/4997/#review4161 ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > + ASSERT_LE(4, pipeline->rtp_packets_sent()) > + << "Local track " << trackId << " isn't sending RTP"; > + ASSERT_LE(1, pipeline->rtcp_packets_received()) > + << "Local track " << trackId << " isn't receiving RTCP"; You aren't checking packet send counts on the video side here. ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > + ASSERT_LE(4, pipeline->rtp_packets_received()) > + << "Remote track " << trackId << " isn't receiving RTP"; > + ASSERT_LE(1, pipeline->rtcp_packets_sent()) > + << "Remote track " << trackId << " isn't sending RTCP"; As above. ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > - ASSERT_GE(a1_->GetPacketsSent(0), 40); > + ASSERT_GE(a1_->GetPacketsSent(0), 4); > // In this case we should receive nothing. > ASSERT_EQ(a2_->GetPacketsReceived(0), 0); Can these be switched to something identifying the stream ID rather than index? ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > + int GetPacketsReceived(size_t stream) const { I saw just one instance where this was called. Inlining it in the std::string version might be nice. ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > + int GetPacketsSent(size_t stream) const { Ditto. ::: media/webrtc/signaling/test/signaling_unittests.cpp (Diff revision 1) > + void CheckStreams() Is the fact that this is only checking audio a problem? Also, the two blocks in this function are wholly duplicated.

Comment on attachment 8574191 [details] MozReview Request: bz://1136252/bwc https://reviewboard.mozilla.org/r/4997/#review4195 Ship It!

https://reviewboard.mozilla.org/r/4997/#review4197 > You aren't checking packet send counts on the video side here. If I recall correctly, no video ends up flowing because we cannot do fake video in this test yet. > Can these be switched to something identifying the stream ID rather than index? I suppose this could be re-written to find the identifiers and use those. > Is the fact that this is only checking audio a problem? > > Also, the two blocks in this function are wholly duplicated. Yeah, I can clean that up.

(In reply to Byron Campen [:bwc] from comment #18) > https://reviewboard.mozilla.org/r/4997/#review4197 > > > You aren't checking packet send counts on the video side here. > > If I recall correctly, no video ends up flowing because we cannot do fake > video in this test yet. OK, a comment to that effect might be wise.

https://reviewboard.mozilla.org/r/4997/#review4231 > I suppose this could be re-written to find the identifiers and use those. Looked into this, and modifying would just end up being the same amount of code, moved into a less-common place. > If I recall correctly, no video ends up flowing because we cannot do fake video in this test yet. Bug filed (1142320) and comment added.

Comment on attachment 8574191 [details] MozReview Request: bz://1136252/bwc /r/4999 - Bug 1136252 - Part 1: Increase the number of instances of signaling_unittests that can be run in parallel. r=mt /r/5001 - Bug 1136252 - Part 2: Wait for less RTP in signaling_unittests. r=mt /r/5003 - Bug 1136252 - Part 3: Remove some unnecessary sleeps in signaling_unittests. r=mt /r/5005 - Bug 1136252 - Part 4: Remove/consolidate code in signaling_unittests. Includes removing most SDP checks, since that belongs in jsep_session_unittest. r=mt /r/5007 - Bug 1136252 - Part 5: Fix bug where candidates could be trickled before setRemote during renegotiation. r=mt /r/5009 - Bug 1136252 - Part 6: Extend timeouts for RTP/RTCP, until bug 1137948 can be fixed. r=mt Pull down these commits: hg pull review -r 0f53df6af5e22ca2023eb4e5a020f62f359d1ea6

Comment on attachment 8574191 [details] MozReview Request: bz://1136252/bwc Carry forward r=mt https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9c890c4ff2a

Seeing something that looks like infra bustage on that last try that retriggers aren't helping with. Let's see if a rebase clears things up. https://treeherder.mozilla.org/#/jobs?repo=try&revision=572cccf7f73c

Looks like try is just hosed on OS 10.6 right now. Doing some retriggers on other infra errors, and will land without the 10.6 builds and hope for the best.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ec8bb1c0cf8 https://hg.mozilla.org/integration/mozilla-inbound/rev/77605be85deb https://hg.mozilla.org/integration/mozilla-inbound/rev/00ee5c75c36b https://hg.mozilla.org/integration/mozilla-inbound/rev/6519a9e7d3d8 https://hg.mozilla.org/integration/mozilla-inbound/rev/856f851dcb5d https://hg.mozilla.org/integration/mozilla-inbound/rev/8878cee7f4fc

https://hg.mozilla.org/mozilla-central/rev/4ec8bb1c0cf8 https://hg.mozilla.org/mozilla-central/rev/77605be85deb https://hg.mozilla.org/mozilla-central/rev/00ee5c75c36b https://hg.mozilla.org/mozilla-central/rev/6519a9e7d3d8 https://hg.mozilla.org/mozilla-central/rev/856f851dcb5d https://hg.mozilla.org/mozilla-central/rev/8878cee7f4fc