Remove test-level assert property from tests #14409

qiuzhong · 2018-12-07T05:41:02Z

test-level asserts is the only remaining use of is for test-level
assert metadata. Remove all the assert property from tests and mark
them as test comments if possible.
Related: #14394

test-level asserts is the only remaining use of is for test-level assert metadata. Remove all the assert property from tests and mark them as test comments if possible. Related: web-platform-tests#14394

foolip · 2018-12-07T19:36:47Z

common/performance-timeline-utils.js

@@ -10,7 +10,8 @@ function wp_test(func, msg, properties)
    if (performanceNamespace === undefined || performanceNamespace == null)
    {
      // show a single error that window.performance is undefined
-      test(function() { assert_true(performanceNamespace !== undefined && performanceNamespace != null, "window.performance is defined and not null"); }, "window.performance is defined and not null.", {author:"W3C http://www.w3.org/",help:"http://www.w3.org/TR/navigation-timing/#sec-window.performance-attribute",assert:"The window.performance attribute provides a hosting area for performance related attributes. "});
+      // The window.performance attribute provides a hosting area for performance related attributes.
+      test(function() { assert_true(performanceNamespace !== undefined && performanceNamespace != null, "window.performance is defined and not null"); }, "window.performance is defined and not null.", {author:"W3C http://www.w3.org/",help:"http://www.w3.org/TR/navigation-timing/#sec-window.performance-attribute"});


For help and author may I suggest moving the information to the top of the test files using the existing style for that? Otherwise these same tests will need to be updated again to remove the properties entirely.

@foolip , I move them to the head of test files.

For `help` and `author`, moving the information to the top of the test files. Related: web-platform-tests#14394

foolip

LGTM % single nit. I'll push a commit to fix that.

foolip · 2018-12-10T09:17:35Z

css/cssom/style-sheet-interfaces-001.html

@@ -4,6 +4,10 @@
  <title>CSS Test: CSSOM StyleSheet Initial Values</title>
  <link rel="author" title="Bear Travis" href="mailto:[email protected]">
  <link rel="help" href="http://www.w3.org/TR/cssom-1/#css-style-sheets">
+  <link rel="help" href="http://www.w3.org/TR/cssom-1/#the-linkstyle-interface">
+  <link rel="help" href="https://www.w3.org/TR/cssom-1/#the-linkstyle-interface">
+  <link rel="help" href="http://www.w3.org/TR/cssom-1/#css-style-sheets">


This line was already present above.

foolip · 2018-12-10T09:47:12Z

There's some preexisting flakiness here:

https://tools.taskcluster.net/groups/POzAsNBJQd6tqMf6yT_yag/tasks/GJDhtVt_QzaLfLIj2SBRlA/details:

Test	Subtest	Results	Messages
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/empty.py?id=audio-source-mpeg is expected to have initiatorType audio`	PASS: 9/10, MISSING: 1/10
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/empty.py?id=beacon is expected to have initiatorType beacon`	PASS: 6/10, MISSING: 4/10
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/empty.py?id=video-track is expected to have initiatorType track`	PASS: 9/10, MISSING: 1/10
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/empty.py?id=audio-source-ogg is expected to have initiatorType audio`	PASS: 2/10, MISSING: 8/10
`/resource-timing/resource_reparenting.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=move_to_child is expected to have initiatorType img`	PASS: 6/10, MISSING: 4/10
`/resource-timing/resource_reparenting.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=move_to_child is expected to be in the Resource Timing buffer`	FAIL: 4/10, MISSING: 6/10	`assert_unreached: Reached unreachable code`

https://tools.taskcluster.net/groups/POzAsNBJQd6tqMf6yT_yag/tasks/PHx9hP8jQM2zOPeisrZ4Tw/details

Test	Subtest	Results	Messages
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/fonts/Ahem.ttf is expected to have initiatorType css`	FAIL: 1/10, MISSING: 9/10	`assert_equals: http://web-platform.test:8000/fonts/Ahem.ttf is expected to have initiatorType css expected "css" but got "other"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/fonts/Ahem.ttf?id=n1 is expected to have initiatorType css`	FAIL: 1/10, MISSING: 9/10	`assert_equals: http://web-platform.test:8000/fonts/Ahem.ttf?id=n1 is expected to have initiatorType css expected "css" but got "other"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/media/test.ogv?id=video-source is expected to have initiatorType video`	FAIL: 2/10, MISSING: 8/10	`assert_equals: http://web-platform.test:8000/media/test.ogv?id=video-source is expected to have initiatorType video expected "video" but got "other"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=1 is expected to have initiatorType css`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=1 is expected to have initiatorType css expected "css" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=2 is expected to have initiatorType css`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=2 is expected to have initiatorType css expected "css" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=cursor is expected to have initiatorType css`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=cursor is expected to have initiatorType css expected "css" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=input is expected to have initiatorType input`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=input is expected to have initiatorType input expected "input" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=n1 is expected to have initiatorType css`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=n1 is expected to have initiatorType css expected "css" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/blue.png?id=svg-image is expected to have initiatorType image`	FAIL: 9/10, PASS: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/blue.png?id=svg-image is expected to have initiatorType image expected "image" but got "img"`
`/resource-timing/resource_initiator_types.html`	`http://web-platform.test:8000/resource-timing/resources/nested.css?id=prefetch is expected to have initiatorType link`	FAIL: 9/10, MISSING: 1/10	`assert_equals: http://web-platform.test:8000/resource-timing/resources/nested.css?id=prefetch is expected to have initiatorType link expected "link" but got "other"`
`/user-timing/measure_associated_with_navigation_timing.html`	`Second measure of current mark to navigationStart should be negative value.`	FAIL: 1/10, PASS: 9/10	`assert_true: Second measure of current mark to navigationStart should be negative value. expected true got false`
`/user-timing/measure_associated_with_navigation_timing.html`	`Measure from domComplete event to most recent mark "a" should have longer duration.`	FAIL: 1/10, PASS: 9/10	`assert_true: Measure from domComplete event to most recent mark "a" should have longer duration. expected true got false`
`/user-timing/measure_associated_with_navigation_timing.html`	`Measure from most recent mark to navigationStart should have longer duration.`	FAIL: 1/10, PASS: 9/10	`assert_true: Measure from most recent mark to navigationStart should have longer duration. expected true got false`

foolip · 2018-12-10T09:53:12Z

I've filed #14433 and #14434 about flaky tests.

foolip · 2018-12-10T12:33:03Z

Diff of runs before/after this change:
https://wpt.fyi/results/?label=taskcluster&product=chrome@226f3b50a6&product=chrome@9f2b7133fa&diff
https://wpt.fyi/results/?label=taskcluster&product=firefox@226f3b50a6&product=firefox@9f2b7133fa&diff

There are apparent regressions, but not in the directories changed, so I suspect this was flakiness.

@lukebjerring FYI if you're keeping track of people/occurrences of people trying to get this info.

Remove test-level assert property from tests

fa019ce

test-level asserts is the only remaining use of is for test-level assert metadata. Remove all the assert property from tests and mark them as test comments if possible. Related: web-platform-tests#14394

wpt-pr-bot added assets css-transitions css-values css-variables cssom cssom-view hr-time mediaqueries navigation-timing requestidlecallback resource-timing user-timing wg-css wg-webperf labels Dec 7, 2018

wpt-pr-bot assigned fantasai Dec 7, 2018

wpt-pr-bot requested review from AutomatedTester, dbaron, deniak, dontcallmedom, fantasai, farre, FremyCompany, frivoal, gsnedders, haoxli, igrigorik, lilles, plehegar and plinss December 7, 2018 05:41

wpt-pr-bot requested a review from rmcilroy December 7, 2018 05:41

foolip reviewed Dec 7, 2018

View reviewed changes

qiuzhong force-pushed the remove-test-level-assert branch from 1591fe5 to 78aa08d Compare December 10, 2018 06:26

Move help and author properties to head

78aa08d

For `help` and `author`, moving the information to the top of the test files. Related: web-platform-tests#14394

foolip approved these changes Dec 10, 2018

View reviewed changes

consolidate help links

78c0a07

This was referenced Dec 10, 2018

Many Resource Timing tests are flaky #14433

Open

Some User Timing tests are flaky #14434

Open

foolip merged commit 9f2b713 into web-platform-tests:master Dec 10, 2018

foolip mentioned this pull request Dec 10, 2018

Remove testharness.js test-level asserts #14394

Open

jugglinmike mentioned this pull request Dec 12, 2018

Taskcluster status checks not finishing and blocking PRs #14165

Closed

yoavweiss mentioned this pull request Dec 20, 2018

[Resource-Timing] Reduce test flakiness #14616

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove test-level assert property from tests #14409

Remove test-level assert property from tests #14409

qiuzhong commented Dec 7, 2018

foolip Dec 7, 2018

qiuzhong Dec 10, 2018

foolip left a comment

foolip Dec 10, 2018

foolip commented Dec 10, 2018

foolip commented Dec 10, 2018

foolip commented Dec 10, 2018

Remove test-level assert property from tests #14409

Remove test-level assert property from tests #14409

Conversation

qiuzhong commented Dec 7, 2018

foolip Dec 7, 2018

Choose a reason for hiding this comment

qiuzhong Dec 10, 2018

Choose a reason for hiding this comment

foolip left a comment

Choose a reason for hiding this comment

foolip Dec 10, 2018

Choose a reason for hiding this comment

foolip commented Dec 10, 2018

foolip commented Dec 10, 2018

foolip commented Dec 10, 2018