Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add some tests that check SDP after transceiver stop #14423

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Add some tests that check SDP after transceiver stop #14423

merged 1 commit into from
Jan 17, 2019

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Dec 7, 2018

No description provided.

@foolip
Copy link
Member

foolip commented Dec 20, 2018

This PR is blocked on the required "Travis CI - Pull Request" check after #14499. In order to trigger it, I will close and reopen this PR.

@foolip foolip closed this Dec 20, 2018
@foolip foolip reopened this Dec 20, 2018
@alvestrand
Copy link
Contributor

Needs more review, but very good to have.

@youennf youennf force-pushed the transceiver-stop-sdp branch from f7b8b7a to bb1da75 Compare January 10, 2019 17:32
@youennf
Copy link
Contributor Author

youennf commented Jan 10, 2019

Updated the tests according review and added some more checks.
As per my reading of https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-24#section-5.2, the first test is correct.
Firefox and Safari behaves the same for all tests except the 3rd one where Firefox always set the m section as inactive.

henbos
henbos approved these changes Jan 14, 2019
View reviewed changes
Copy link
Contributor

@henbos henbos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@youennf youennf merged commit adc0811 into web-platform-tests:master Jan 17, 2019
docfaraday
docfaraday reviewed Feb 5, 2019
View reviewed changes
webrtc/RTCRtpTransceiver-stop.html

const offer = await pc1.createOffer();

assert_true(offer.sdp.includes("a=sendonly"), "The audio m-section should be sendonly");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@docfaraday docfaraday docfaraday left review comments

@alvestrand alvestrand alvestrand left review comments

@henbos henbos henbos approved these changes

@agouaillard agouaillard Awaiting requested review from agouaillard

@dontcallmedom dontcallmedom Awaiting requested review from dontcallmedom

@guidou guidou Awaiting requested review from guidou

@jan-ivar jan-ivar Awaiting requested review from jan-ivar

@phoglund phoglund Awaiting requested review from phoglund

@rwaldron rwaldron Awaiting requested review from rwaldron

@snuggs snuggs Awaiting requested review from snuggs

Assignees

@snuggs snuggs

Labels
webrtc wg-webrtc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@youennf @foolip @alvestrand @docfaraday @henbos @snuggs @wpt-pr-bot