Closed Bug 1208176 Opened 4 years ago Closed 4 years ago

ice_unittests is broken, again

Categories

(Core :: WebRTC: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed
Blocking Flags:

People

(Reporter: bwc, Assigned: bwc)

Details

Attachments

(3 files)

Tested on my linux box. Seems to be related to prflx showing up where we did not expect.

[  FAILED  ] 9 tests, listed below:
[  FAILED  ] IceConnectTest.TestConnect
[  FAILED  ] IceConnectTest.TestConnectBothControllingP1Wins
[  FAILED  ] IceConnectTest.TestConnectBothControllingP2Wins
[  FAILED  ] IceConnectTest.TestConnectIceLiteOfferer
[  FAILED  ] IceConnectTest.TestConnectTwoComponents
[  FAILED  ] IceConnectTest.TestConnectTwoComponentsDisableSecond
[  FAILED  ] IceConnectTest.TestConnectAutoPrioritize
[  FAILED  ] IceConnectTest.TestSendReceive
[  FAILED  ] IceConnectTest.TestPollCandPairsAfterConnect
Similar problems occasionally show up for TCP ice unit tests, which I though so far were limited to TCP.
backlog: --- → webrtc/webaudio+
Rank: 25
Priority: -- → P2
Assignee: nobody → docfaraday
This seems to only be a problem on my linux box, due to some extra virbr interfaces, and linux's tendency to play it fast and loose when sending traffic to yourself. The patch from bug 1186590 perturbed things just enough to expose the problem, since it caused the virbr interface to be prioritized over the real one.

One thing that I could do is make the test-cases accept prflx as a general rule. However, this might mask problems with trickle, since one side could fail to trickle anything and stuff would still work. I would also have to add a one-sided trickle test (which I suppose we should have anyway).

Another option is to add my weird "real" interface name to the priority list, which probably makes sense to do anyway, although it is only a band-aid.
Bug 1208176 - Part 1: Add a couple of interface names.
Attachment #8670456 - Flags: review?(drno)
Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest.
Attachment #8670457 - Flags: review?(drno)
Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest.
Attachment #8670458 - Flags: review?(drno)
Attachment #8670456 - Flags: review?(drno) → review+
Comment on attachment 8670456 [details]
MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno

https://reviewboard.mozilla.org/r/21375/#review19279
Comment on attachment 8670457 [details]
MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

https://reviewboard.mozilla.org/r/21377/#review19283

::: media/mtransport/test/ice_unittest.cpp:2513
(Diff revision 1)
> +TEST_F(IceConnectTest, OneSidedTrickle) {
> +  AddStream("first", 1);
> +  ASSERT_TRUE(Gather());
> +  ConnectTrickle();
> +  RealisticTrickleDelay(p1_->ControlTrickle(0));
> +  DropTrickleCandidates(p2_->ControlTrickle(0));
> +  ASSERT_TRUE_WAIT(p1_->ice_complete(), 1000);
> +  ASSERT_TRUE_WAIT(p2_->ice_complete(), 1000);
> +}

Just to be safe: how about the reverse p1 dropping everything?
Attachment #8670457 - Flags: review?(drno) → review+
Comment on attachment 8670458 [details]
MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

https://reviewboard.mozilla.org/r/21379/#review19285
Attachment #8670458 - Flags: review?(drno) → review+
Attachment #8670456 - Attachment description: MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. → MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno
Comment on attachment 8670456 [details]
MozReview Request: Bug 1208176 - Part 1: Add a couple of interface names. r=drno

Bug 1208176 - Part 1: Add a couple of interface names. r=drno
Comment on attachment 8670457 [details]
MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno

Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno
Attachment #8670457 - Attachment description: MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. → MozReview Request: Bug 1208176 - Part 2: Add a one-sided trickle test case to ice_unittest. r=drno
Comment on attachment 8670458 [details]
MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno

Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno
Attachment #8670458 - Attachment description: MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. → MozReview Request: Bug 1208176 - Part 3: Be forgiving when we see prflx instead of host candidates in ice_unittest. r=drno
Check back on try.
Flags: needinfo?(docfaraday)
Flags: needinfo?(docfaraday)
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.