Closed
Bug 1220441
Opened 7 years ago
Closed 7 years ago
ice unit tests TestStun[Tcp]ServerTrickle are at least unreliable
Categories
(Core :: WebRTC: Networking, defect, P2)
Core
WebRTC: Networking
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
backlog | webrtc/webaudio+ |
People
(Reporter: drno, Assigned: drno)
References
Details
Attachments
(1 file)
I'm not quite sure what the TestStun[Tcp]ServerTrickle tests are suppose to prove. We configure the fake STUN server to throw away incoming packets... TestStunServer::GetInstance(AF_INET)->SetActive(false); We start gathering. Gather(0); Now we assert that the round trip time between our stun client and the fake stun server is bigger then the execution time between the return of the previous statement and the next assert?!?! ASSERT_FALSE(StreamHasMatchingCandidate(0, "192.0.2.1")); Now we let the fake stun server answer requests... TestStunServer::GetInstance(AF_INET)->SetActive(true); WaitForGather(); ASSERT_TRUE(StreamHasMatchingCandidate(0, "192.0.2.1")); I guess the point is suppose to be that the stun client keeps retransmitting its requests until it gets a response (and we hopefully don't time out). Problem is: for TCP there are no retransmits. So if the fake STUN server was set to inactive at the time of receiving the first request it drops the request and then will never send a response! For UDP one possible improvement could be to tell the STUN server how many requests to throw away before answering, if that is what we want to test here. For TCP I could only see us delaying the answer here so that the gathering will take time until it finishes.
Assignee | ||
Comment 1•7 years ago
|
||
Byron: any thoughts/ideas?
backlog: --- → webrtc/webaudio+
Rank: 22
status-firefox45:
affected → ---
Flags: needinfo?(docfaraday)
Priority: -- → P2
Comment 2•7 years ago
|
||
I think your analysis is correct, and this this test makes no sense for TCP.
Flags: needinfo?(docfaraday)
Assignee | ||
Comment 3•7 years ago
|
||
Bug 1220441: Improve gather trickle ice unit tests
Assignee | ||
Comment 4•7 years ago
|
||
Comment on attachment 8682205 [details] MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf
Attachment #8682205 -
Attachment description: MozReview Request: Bug 1220441: Improve gather trickle ice unit tests → MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf
Attachment #8682205 -
Flags: review?(mfroman)
Attachment #8682205 -
Flags: review?(docfaraday)
Assignee | ||
Comment 5•7 years ago
|
||
Just looking for one review. So whoever can get this done quicker... :-)
Updated•7 years ago
|
Attachment #8682205 -
Flags: review?(mfroman)
Comment 6•7 years ago
|
||
Comment on attachment 8682205 [details] MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf https://reviewboard.mozilla.org/r/23989/#review21583 ::: media/mtransport/test/stunserver.cpp:479 (Diff revision 2) > server->Process(reinterpret_cast<const uint8_t *>(message), message_len, Looks like this can now be called even if !server->active. Is this intended?
Assignee | ||
Comment 7•7 years ago
|
||
Comment on attachment 8682205 [details] MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23989/diff/2-3/
Attachment #8682205 -
Flags: review?(mfroman)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → drno
Comment 8•7 years ago
|
||
Comment on attachment 8682205 [details] MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf https://reviewboard.mozilla.org/r/23989/#review21893
Attachment #8682205 -
Flags: review?(docfaraday) → review+
Assignee | ||
Updated•7 years ago
|
Attachment #8682205 -
Flags: review?(mfroman)
Comment 10•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/27066ece22a9
Keywords: checkin-needed
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/27066ece22a9
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment 12•7 years ago
|
||
Comment on attachment 8682205 [details] MozReview Request: Bug 1220441: Improve gather trickle ice unit tests. r?bwc,mjf https://reviewboard.mozilla.org/r/23989/#review30909
Attachment #8682205 -
Flags: review+
You need to log in
before you can comment on or make changes to this bug.
Description
•