search

web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
4.89k stars 3.05k forks source link

Remove testharness.js test-level asserts #14394

Open foolip opened 5 years ago

foolip commented 5 years ago

After https://github.com/web-platform-tests/wpt/issues/11120, test-level asserts is the only remaining use of test-level metadata. Example: https://github.com/web-platform-tests/wpt/blob/8bb0b769af8d7a6d245db31c0b6029eb3b7da861/hr-time/monotonic-clock.any.js#L1-L3

https://chromium-review.googlesource.com/c/chromium/src/+/1070155 identified tests that use this and will need updating before the feature is removed:

/css/css-transitions/currentcolor-animation-001.html
/css/css-values/calc-unit-analysis.html
/css/css-variables/test_variable_legal_values.html
/css/cssom/computed-style-001.html
/css/cssom/computed-style-005.html
/css/cssom/inline-style-001.html
/css/cssom/medialist-interfaces-001.html
/css/cssom/medialist-interfaces-002.html
/css/cssom/medialist-interfaces-003.html
/css/cssom/medialist-interfaces-004.html
/css/cssom/style-sheet-interfaces-001.html
/css/cssom/style-sheet-interfaces-002.html
/css/cssom-view/offsetParent_element_test.html
/css/mediaqueries/test_media_queries.html
/hr-time/monotonic-clock.any.js
/navigation-timing/test_document_open.html
/navigation-timing/test_navigate_within_document.html
/navigation-timing/test_navigation_attributes_exist.html
/navigation-timing/test_navigation_redirectCount_none.html
/navigation-timing/test_navigation_type_backforward.html
/navigation-timing/test_navigation_type_enums.html
/navigation-timing/test_navigation_type_reload.html
/navigation-timing/test_no_previous_document.html
/navigation-timing/test_performance_attributes_exist.html
/navigation-timing/test_performance_attributes_exist_in_object.html
/navigation-timing/test_readwrite.html
/navigation-timing/test_timing_attributes_exist.html
/navigation-timing/test_timing_attributes_order.html
/navigation-timing/test_timing_client_redirect.html
/navigation-timing/test_timing_reload.html
/navigation-timing/test_timing_server_redirect.html
/navigation-timing/test_timing_xserver_redirect.html
/navigation-timing/test_unique_performance_objects.html
/requestidlecallback/basic.html
/resource-timing/resource_TAO_cross_origin_redirect_chain.html
/resource-timing/resource_TAO_match_origin.htm
/resource-timing/resource_TAO_match_wildcard.htm
/resource-timing/resource_TAO_multi.htm
/resource-timing/resource_TAO_multi_wildcard.html
/resource-timing/resource_TAO_null.htm
/resource-timing/resource_TAO_origin.htm
/resource-timing/resource_TAO_origin_uppercase.htm
/resource-timing/resource_TAO_space.htm
/resource-timing/resource_TAO_wildcard.htm
/resource-timing/resource_TAO_zero.htm
/resource-timing/resource_cached.htm
/resource-timing/resource_connection_reuse.html
/resource-timing/resource_dynamic_insertion.html
/resource-timing/resource_timing_cross_origin_redirect_chain.html
/user-timing/measure.html
/user-timing/measure_navigation_timing.html

Some false positives due to flakiness is possible.

foolip commented 5 years ago

Most of the asserts probably should not just be removed, but rather kept as comments in the tests.

qiuzhong commented 5 years ago

@foolip , if you don't mind I'll take this issue.

foolip commented 5 years ago

Perfect, thank you!

foolip commented 5 years ago

When merging https://github.com/web-platform-tests/wpt/pull/14409 I tweaked the commit message to close this issue. That was a mistake, because actually there's remaining work to do here. Once the changes have been imported into Chromium I'll restart https://chromium-review.googlesource.com/c/chromium/src/+/1070155 and if that looks good we can remove the test-level properties from testharness.js altogether.

foolip commented 5 years ago

https://chromium-review.googlesource.com/c/chromium/src/+/1070155 has revealed a bunch of tests that use the properties dictionary and that should be inspected and probably changed before we remove support for it:

/css/css-fonts/format-specifiers-variations.html
/css/css-shapes/shape-outside/values/shape-margin-000.html
/css/css-shapes/shape-outside/values/shape-margin-001.html
/css/css-shapes/shape-outside/values/shape-margin-002.html
/css/css-transitions/transition-delay-001.html
/css/css-transitions/transition-duration-001.html
/css/css-transitions/transition-timing-function-001.html
/html/infrastructure/urls/resolving-urls/query-encoding/utf-16be.html
/html/infrastructure/urls/resolving-urls/query-encoding/utf-16le.html
/html/infrastructure/urls/resolving-urls/query-encoding/utf-8.html
/html/infrastructure/urls/resolving-urls/query-encoding/windows-1251.html
/html/infrastructure/urls/resolving-urls/query-encoding/windows-1252.html
/html/semantics/forms/the-input-element/datetime.html
/html/semantics/forms/the-input-element/range.html
/html/semantics/forms/the-input-element/search_input.html
/html/semantics/scripting-1/the-template-element/template-element/template-as-a-descendant.html
/html/syntax/parsing/template/clearing-the-stack-back-to-a-given-context/clearing-stack-back-to-a-table-body-context.html
/html/syntax/parsing/template/clearing-the-stack-back-to-a-given-context/clearing-stack-back-to-a-table-context.html
/html/syntax/parsing/template/clearing-the-stack-back-to-a-given-context/clearing-stack-back-to-a-table-row-context.html
/html/syntax/parsing/template/creating-an-element-for-the-token/template-owner-document.html
/media-source/mediasource-activesourcebuffers.html
/media-source/mediasource-addsourcebuffer-mode.html
/media-source/mediasource-append-buffer.html
/media-source/mediasource-appendbuffer-quota-exceeded.html
/media-source/mediasource-appendwindow.html
/media-source/mediasource-changetype-play.html
/media-source/mediasource-changetype.html
/media-source/mediasource-closed.html
/media-source/mediasource-config-change-webm-a-bitrate.html
/media-source/mediasource-config-change-webm-av-audio-bitrate.html
/media-source/mediasource-config-change-webm-av-framesize.html
/media-source/mediasource-config-change-webm-av-video-bitrate.html
/media-source/mediasource-config-change-webm-v-bitrate.html
/media-source/mediasource-config-change-webm-v-framerate.html
/media-source/mediasource-config-change-webm-v-framesize.html
/media-source/mediasource-detach.html
/media-source/mediasource-duration-boundaryconditions.html
/media-source/mediasource-endofstream-invaliderror.html
/media-source/mediasource-errors.html
/media-source/mediasource-liveseekable.html
/media-source/mediasource-multiple-attach.html
/media-source/mediasource-play-then-seek-back.html
/media-source/mediasource-play.html
/media-source/mediasource-redundant-seek.html
/media-source/mediasource-removesourcebuffer.html
/media-source/mediasource-seek-beyond-duration.html
/media-source/mediasource-seek-during-pending-seek.html
/media-source/mediasource-seekable.html
/media-source/mediasource-sourcebuffer-mode.html
/media-source/mediasource-sourcebuffer-trackdefaults.html
/media-source/mediasource-sourcebufferlist.html
/media-source/mediasource-timestamp-offset.html
qiuzhong commented 5 years ago

@foolip , so the goal is to remove all the test-level properties support from testharness.js?

I noticed you raise Error on purpose for the test, async_test, promise_test and generate_tests if the test-level properties are set. Should they be removed from all the test files?

foolip commented 5 years ago

When I looked through some of the list I saw some cases that aren't so obvious what to do about. I think we'll just have to remove the uses that aren't doing anything useful, and then look at what remains and say if we should keep the properties dictionary or not.

qiuzhong commented 5 years ago

New PR filed for removing the these useless properties, including:

The properties dictionary remains in such case: properties is used explicitly testing function body of promise_test's func parameter.