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.

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

Add negative tests for prefixed variants of MediaStream, Speech and WebRTC #7507

Merged
merged 4 commits into from
Sep 27, 2017

Conversation

foolip
Copy link
Member

@foolip foolip commented Sep 27, 2017

No description provided.

@foolip
Copy link
Member Author

foolip commented Sep 27, 2017

@lukebjerring, this is a typical case where #7475 would really increase confidence in the changes. I've written the tests by copy-pasting the API names from various places, but any typo would cause the tests to pass. Being able to easily see what the pass/fail status of all new tests are would make it clear at a glance if I got it right, since I know which of these are in Chrome (all of them).

@drufball, if you're doing a PRD for this work, maybe this can be one of the use cases. The other big one is of course when changing tests.

@foolip
Copy link
Member Author

foolip commented Sep 27, 2017

@andrenatal @gshires, I added you as OWNERS of speech-api since this is the first test there. Are you OK with that?

@ghost
Copy link

ghost commented Sep 27, 2017

Build PASSED

Started: 2017-09-27 21:54:55
Finished: 2017-09-27 21:59:19

View more information about this build on:

foolip added a commit that referenced this pull request Sep 27, 2017
Tests should follow what the specs say, and unless the specs define
the prefixed APIs [1] the right this is to assume the per-spec names
and to add historical.html tests asserting the absense of prefixed
APIs that have existed somewhere.

Based on web-confluence.appspot.com, navigator.vibrate was never
prefixed, but mozSrcObject is in Firefox. A test is added for
mozSrcObject, and the others APIs are already covered by existing
tests, or one of these:
#7507
#7508

[1] https://dom.spec.whatwg.org/#dom-element-webkitmatchesselector
    https://url.spec.whatwg.org/#url-class
    and others
rwaldron
rwaldron previously approved these changes Sep 27, 2017
@rwaldron rwaldron dismissed their stale review September 27, 2017 17:38

Need to make change requests


test(function() {
assert_false("webkitRTCPeerConnection" in window);
}, "webkitRTCPeerConnection interface should not exist");
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you've add mozGetUserMedia in mediacapture-streams/historical.html, should this file also include:

mozRTCIceCandidate
mozRTCPeerConnection
mozRTCSessionDescription

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for sure! I didn't know they existed :)

@foolip
Copy link
Member Author

foolip commented Sep 27, 2017

@fleizach, added you here, now maybe you can review? :)

Copy link

@fleizach fleizach left a comment

Choose a reason for hiding this comment

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

approved speech-api owners change

@foolip foolip dismissed rwaldron’s stale review September 27, 2017 21:58

feedback addressed

@foolip foolip merged commit f38b775 into master Sep 27, 2017
@foolip foolip deleted the more-historical-tests branch September 27, 2017 22:06
foolip added a commit that referenced this pull request Sep 29, 2017
Tests should follow what the specs say, and unless the specs define
the prefixed APIs [1] the right this is to assume the per-spec names
and to add historical.html tests asserting the absense of prefixed
APIs that have existed somewhere.

Based on web-confluence.appspot.com, navigator.vibrate was never
prefixed, but mozSrcObject is in Firefox. A test is added for
mozSrcObject, and the others APIs are already covered by existing
tests, or one of these:
#7507
#7508

[1] https://dom.spec.whatwg.org/#dom-element-webkitmatchesselector
    https://url.spec.whatwg.org/#url-class
    and others
@foolip
Copy link
Member Author

foolip commented Sep 29, 2017

@bobholt, when I was showing this to @drufball I noticed that https://pulls.web-platform-tests.org/build/18921 didn't have results for some of the browsers, I think it said "No tests run" or something. But now I'm looking again and they're there. This was all today, well after "Build PASSED" was posted. Any idea what might be going on?

@bobholt
Copy link
Contributor

bobholt commented Oct 2, 2017 via email

jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this pull request Nov 16, 2017
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this pull request Nov 16, 2017
Tests should follow what the specs say, and unless the specs define
the prefixed APIs [1] the right this is to assume the per-spec names
and to add historical.html tests asserting the absense of prefixed
APIs that have existed somewhere.

Based on web-confluence.appspot.com, navigator.vibrate was never
prefixed, but mozSrcObject is in Firefox. A test is added for
mozSrcObject, and the others APIs are already covered by existing
tests, or one of these:
web-platform-tests#7507
web-platform-tests#7508

[1] https://dom.spec.whatwg.org/#dom-element-webkitmatchesselector
    https://url.spec.whatwg.org/#url-class
    and others
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants