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

Remove options parameter from mediasource. #14639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qiuzhong
Copy link
Contributor

Part of removing the support of test-level properties from testharness.js

Part of removing the support of test-level properties from testharness.js
@qiuzhong
Copy link
Contributor Author

Please don't merge this before #14519 is landed.

@wolenetz
Copy link
Member

LGTM. Downstream Chromium/Blink usage of testharness.js in media-source tests still uses test options to customize timeouts, and also there's a mediasource-sourcebufferlist test case in downstream that included a one-off change to overload the options parameter to mediasource_testafterdataloaded in mediasource-util.js to have an "allow_media_element_error" option change the way the util setup the mediasource and element. That element error overload kludginess can be fixed downstream, but I'm uncertain how downstream can maintain custom test case timeouts when #14519 is landed upstream and pulled into downstream. Is there an alternative to the old " { timeout: timeout_in_milliseconds }" option style that I may use downstream, or is such alternative also no longer necessary for some other reason?

@qiuzhong
Copy link
Contributor Author

Is there an alternative to the old " { timeout: timeout_in_milliseconds }" option style that I may use downstream, or is such alternative also no longer necessary for some other reason?

Please refer to #11120 , test-level timeout property was removed from tests and testharness.js will support it any more. So you'd better not use them it in the tests. Generally, testharness.js will set a default timeout value for each test: 10 seconds. If the tests are slow, you can add <meta name=timeout content=long> in the head of the test html files, that will set the timeout to be 60 seconds.

@zcorpan
Copy link
Member

zcorpan commented Nov 22, 2019

#14519 has not landed yet, adding label to indicate the dependency.

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.

4 participants