signaling_unittests regression: check for m=application no longer appropriate

RESOLVED FIXED in Firefox 22

Status

()

Core
WebRTC: Signaling
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ekr, Assigned: jesup)

Tracking

Trunk
mozilla23
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 fixed, firefox23 fixed)

Details

(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
The signaling unit tests now fail:

[ RUN      ] SignalingTest.JustInit
Init Complete
Init Complete
Close
Close
[       OK ] SignalingTest.JustInit (7752 ms)
[ RUN      ] SignalingTest.CreateSetOffer
Init Complete
Init Complete
onCreateOfferSuccess = v=0
o=Mozilla-SIPUA-22.0a1 1882 0 IN IP4 0.0.0.0
s=SIP Call
t=0 0
a=ice-ufrag:806ad0d3
a=ice-pwd:96f6ae15239b07a021a071625e691263
a=fingerprint:sha-256 A1:D5:40:24:02:75:EC:9B:61:B7:FD:65:1D:52:6E:BE:B1:0E:7A:FA:CC:66:CF:84:65:D3:A1:91:82:28:3B:8A
m=audio 53170 RTP/SAVPF 109 0 8 101
c=IN IP4 192.168.129.33
a=rtpmap:109 opus/48000/2
a=ptime:20
a=rtpmap:0 PCMU/8000
a=rtpmap:8 PCMA/8000
a=rtpmap:101 telephone-event/8000
a=fmtp:101 0-15
a=sendrecv
a=candidate:0 1 UDP 2113601791 192.168.129.33 53170 typ host
a=candidate:2 1 UDP 2112946431 172.16.26.1 50187 typ host
a=candidate:4 1 UDP 2112487679 192.168.134.1 49681 typ host
a=candidate:0 2 UDP 2113601790 192.168.129.33 58365 typ host
a=candidate:2 2 UDP 2112946430 172.16.26.1 50103 typ host
a=candidate:4 2 UDP 2112487678 192.168.134.1 51335 typ host
m=video 49258 RTP/SAVPF 120
c=IN IP4 192.168.129.33
a=rtpmap:120 VP8/90000
a=sendrecv
a=candidate:0 1 UDP 2113601791 192.168.129.33 49258 typ host
a=candidate:2 1 UDP 2112946431 172.16.26.1 64319 typ host
a=candidate:4 1 UDP 2112487679 192.168.134.1 61809 typ host
a=candidate:0 2 UDP 2113601790 192.168.129.33 50784 typ host
a=candidate:2 2 UDP 2112946430 172.16.26.1 57626 typ host
a=candidate:4 2 UDP 2112487678 192.168.134.1 57914 typ host

SDPSanityCheck flags for offer = 0x303 SHOULD_SEND_AUDIO SHOULD_RECV_AUDIO SHOULD_SEND_VIDEO SHOULD_RECV_VIDEO
/Users/ekr/dev/mozilla-inbound/media/webrtc/signaling/test/signaling_unittests.cpp:961: Failure
Expected: (sdp.find("m=application")) != (std::string::npos), actual: 18446744073709551615 vs 18446744073709551615
onSetLocalDescriptionSuccess
Close
Close
(Reporter)

Comment 1

5 years ago
assigning to jesup b/c this looks like a result of the data channel changes.
Assignee: nobody → rjesup
(Assignee)

Comment 2

5 years ago
Created attachment 731999 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation
(Assignee)

Comment 3

5 years ago
Comment on attachment 731999 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation

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

::: media/webrtc/signaling/test/signaling_unittests.cpp
@@ +1219,3 @@
>  }
>  
>  TEST_F(SignalingTest, CreateOfferNoDataChannel)

Note: this test is pretty meaningless now, and likely should be removed.  If the other test worked, we could if we wanted test that it supresses m=application.

Better would be to remove the test, and remove the MozDontOfferDataChannel constraint.
Attachment #731999 - Flags: review?(ekr)
(Assignee)

Comment 4

5 years ago
Created attachment 732048 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation
(Assignee)

Updated

5 years ago
Attachment #731999 - Attachment is obsolete: true
Attachment #731999 - Flags: review?(ekr)
(Assignee)

Comment 5

5 years ago
Created attachment 732049 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation
(Assignee)

Updated

5 years ago
Attachment #732048 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #732049 - Flags: review?(ekr)
(Assignee)

Updated

5 years ago
Whiteboard: [WebRTC] [blocking-webrtc-]
(Reporter)

Comment 6

5 years ago
Comment on attachment 732049 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation

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

lgtm
Attachment #732049 - Flags: review?(ekr) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3391233b3e31
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/3391233b3e31
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
(Assignee)

Updated

5 years ago
status-firefox22: --- → affected
status-firefox23: --- → fixed
Whiteboard: [WebRTC] [blocking-webrtc-] [qa-] → [WebRTC] [blocking-webrtc-] [qa-][webrtc_uplift]
(Assignee)

Comment 9

5 years ago
Comment on attachment 732049 [details] [diff] [review]
Make signaling_unittests understand the changes in datachannel negotiation

[Approval Request Comment]
Bug caused by (feature/regressing bug #): DataChannel SDP landing

User impact if declined: Signaling unit tests will fail on Aurora.  (Note: they aren't run on the test machines currently; we do run them on m-i/m-c on a personal/project continuous integration server/tinderbox.)  Not having this will mean anyone testing on Aurora (especially a patch for uplift) will have to manually apply this patch first.

Testing completed (on m-c, etc.): On m-c for some time and tested in CI

Risk to taking this patch (and alternatives if risky): nil - test code only

String or IDL/UUID changes made by this patch: none
Attachment #732049 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Attachment #732049 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/7f5fc3bf0e76
status-firefox22: affected → fixed
Whiteboard: [WebRTC] [blocking-webrtc-] [qa-][webrtc_uplift] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in before you can comment on or make changes to this bug.