Remove some useless test-level properties #14519
Conversation
qiuzhong
force-pushed
the
remove-useless-test-level-properties
branch
from
218303b to
45601df
Compare
Remove useless test-level properties, including: * Incorrect type of the properties parameter for `promise_test()` * `flags` property passed to `test()`, which is unused in testharness.js * Incorrect type of the properties `generate_tests`: moved to comments * `help` property: moved the head of the test files * Default empty properties dictionary Related: web-platform-tests#14394
| @@ -1,6 +1,34 @@ | |||
| // NOTE: this file needs to be split up rather than expanded. See ../location.sub.html for some | |||
| // extracted tests. Tracked by https://github.com/web-platform-tests/wpt/issues/4934. | |||
|
|
|||
| /* | |||
| * help: | |||
There was a problem hiding this comment.
I've written tests with help but it's likely less useful than a js line comment with a spec link (the only use I imagine is for someone debugging a test).
| @@ -6,6 +6,13 @@ | |||
| <link rel="author" title="Fabrice Clari" href="mailto:[email protected]"> | |||
| <link rel="author" title="Dimitri Bocquet" href="mailto:[email protected]"> | |||
| <link rel="help" href="https://html.spec.whatwg.org/multipage/#the-input-element"> | |||
| <link rel="help" href="https://html.spec.whatwg.org/multipage/#dom-input-type"> | |||
There was a problem hiding this comment.
@Ms2ger you added/edited the spec links in this test. Do you think we should keep them?
media-source/mediasource-util.js
Outdated
| @@ -330,16 +330,15 @@ | |||
| }; | |||
|
|
|||
| window['MediaSourceUtil'] = MediaSourceUtil; | |||
| window['media_test'] = function(testFunction, description, options) | |||
| window['media_test'] = function(testFunction, description) | |||
There was a problem hiding this comment.
@qiuzhong can you separate out just this change? This is one that we definitely should do, since the options aren't used anywhere.
There was a problem hiding this comment.
I reverted the media_test function.
There was a problem hiding this comment.
I mean that these are changes that we should land whether or not we remove properties entirely, so submitting those as a separate PR would be great.
There was a problem hiding this comment.
Sorry, I misunderstood your meaning. This time I reverted the whole test file and I'll file a new PR for it.
It's better to submit a new PR for this file.
|
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. |
|
@dbaron AFAIK, no. I think the main purpose of flagging 'invalid' was to let people use validator tools to help check syntax on their tests; flagging tests with deliberately invalid syntax would let the tools skip those tests. I don't think this logic applies to testharness.js tests, and I'm not aware of any other purpose for this metadata. Maybe @plinss knows more, though. |
|
There were plans to use the per-test metadata in testharness.js tests a long time ago, but those have never developed and are unlikely to at this point. |
|
This can't currently be merged because the wpt-firefox-nightly-stability test is showing unstable results (timing out about 1/3 of the time) on various That seems likely to be an existing problem... I think? |
|
See Yes, these tests are known unstable. I can admin merge this. Thanks @dbaron ! |
Remove useless test-level properties, including:
promise_test()flagsproperty passed totest(), which is unused in testharness.jsgenerate_tests: moved to commentshelpproperty: moved the head of the test filesRelated: Remove testharness.js test-level asserts #14394