Add assert_precondition helper and PRECONDITION_FAILED subtest status by lukebjerring · Pull Request #16689 · web-platform-tests/wpt

lukebjerring · 2019-05-06T17:22:20Z

Prototyping/exploring web-platform-tests/rfcs#16

resources/testharness.js

WebCryptoAPI/generateKey/successes.js

jimsch · 2019-05-08T20:33:11Z

I think that I lost the review flag. If I still had it I would approve.

jgraham · 2019-05-13T13:07:06Z

This needs a testharness unittest, and a test in infrastructure/. I would expect the latter to ERROR until we get changes in mozlog (and maybe wptrunner) to handle the new status. Since we're using mozlog from PyPI this will also block on getting a new mozlog release.

Hexcles · 2019-06-07T22:01:50Z

@jgraham I've fixed a bug and added two tests (infrastructure & resource tests).

jgraham · 2019-07-16T08:21:03Z

My only major concern about this is that it makes it impossible to tell the difference between "this test was disabled by the implementation e.g. for stability issues" and "this test was skipped because the feature is unsupported". That difference seems meaningful to me since you might want to audit tests that are disabled for instability,

lukebjerring · 2019-08-15T20:26:34Z

@jgraham - as agreed, switched the (subtest) status name to PRECONDITION_FAILED.

(Reasons: Using SKIP would be ambiguous with already-supported test-level skippage/disable behaviour, and renaming existing SKIP to something else would be a major headache).

lukebjerring · 2019-08-30T12:59:21Z

@jgraham - I'm seeing the mozlog error we'd expect:

ValueError: Failed to convert value PRECONDITION_FAILED of type str for field status to type SubStatus

https://tools.taskcluster.net/groups/RGFPhLFqRGqvPXAaRXcmGA/tasks/TzNbxq2bTrWslrAGvcfsuA/runs/0/logs/public%2Flogs%2Flive.log#L846

foolip · 2019-09-19T10:51:35Z

WebCryptoAPI/generateKey/successes.js

                }
            }, function(err) {
+                var supported = !err.toString().includes('not supported');
+                assert_precondition(supported, algorithm.name + ' not implemented');


I'll move the bikeshedding here, since I guess this PR will have the docs and testharness.js changes. What will the resulting message in the report be?

Ok, can actually answer this now that I've fixed it. It's the message in the second param to assert_precondition, e.g.

(bad alignment notwithstanding)

lukebjerring · 2019-10-04T19:35:15Z

resources/testharness.js

+    expose(PreconditionFailedError, "PreconditionFailedError");
+
+    PreconditionFailedError.prototype = Object.create(AssertionError.prototype);
+    PreconditionFailedError.prototype.constructor = PreconditionFailedError;


For some reason, new PreconditionFailedError('foo') instanceof AssertionError is false.

The style of inheritence used here looks unfamiliar to me, is it used elsewhere in testharness.js and is instanceof correct in those cases?

It's es5 "inheritance" and yes, AssertionError uses it for inheriting Error

It's overriding Foo.prototype.constructor that looks unfamiliar. Maybe this pattern just isn't right, or is new AssertionError('foo') instanceof Error true?

So, the issue was that the whole thing is in a closure. It uses expose to add the definitions to the global_scope. As a result, the references need to exist before calling expose, otherwise they'll refer to a local of the same name, and the globally resolved AssertionError is a different reference, thus instanceof is false. Fixed by declaring the types in the right order.

lukebjerring · 2019-10-10T14:38:12Z

@jgraham @foolip I've run out of time to get this implemented past the testharness.js layer.

Current state of this PR is that a PRECONDITION_FAILED result is successfully produced when loading the test running on ./wpt serve, but that status is unrecognized (so unrecorded) by the wpt runner / mozlog side.

foolip · 2019-10-10T15:22:26Z

@jgraham where is mozlog maintained? I presume this is a pretty simple fix and that most of the work is in the review and release step...

jgraham · 2019-10-16T09:51:48Z

It's in mozilla-central: https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/

foolip · 2019-10-16T11:42:08Z

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1589056 to track this in mozlog.

foolip · 2019-10-17T12:18:42Z

A review is up at https://phabricator.services.mozilla.com/D49450

foolip · 2019-10-23T07:17:30Z

I've filed #19844 to track this to completion, closing this PR.

This depends on mozlog 5.0 for the new PRECONDITION_FAILED status: https://bugzilla.mozilla.org/show_bug.cgi?id=1589056 Implements web-platform-tests/rfcs#16. Includes parts of #16689. Fixes #19844.

foolip · 2019-11-08T13:32:50Z

Parts of this was landed in #19993 and the test changes are in #20173 + #20176.

…ndition`, a=testonly Automatic update from web-platform-tests [testharness.js] introduce `assert_precondition` This depends on mozlog 5.0 for the new PRECONDITION_FAILED status: https://bugzilla.mozilla.org/show_bug.cgi?id=1589056 Implements web-platform-tests/rfcs#16. Includes parts of web-platform-tests/wpt#16689. Fixes web-platform-tests/wpt#19844. -- [vibration] use `assert_precondition` to avoid spurious pass This demonstrates the use of `assert_precondition` for subtests. -- wpt-commits: 17543bc30bdf65838e9ec8e3b3b6fd210dced66e, a90e89fb031b0b357305ac9d874968d8451b1181 wpt-pr: 19993

…ndition`, a=testonly Automatic update from web-platform-tests [testharness.js] introduce `assert_precondition` This depends on mozlog 5.0 for the new PRECONDITION_FAILED status: https://bugzilla.mozilla.org/show_bug.cgi?id=1589056 Implements web-platform-tests/rfcs#16. Includes parts of web-platform-tests/wpt#16689. Fixes web-platform-tests/wpt#19844. -- [vibration] use `assert_precondition` to avoid spurious pass This demonstrates the use of `assert_precondition` for subtests. -- wpt-commits: 17543bc30bdf65838e9ec8e3b3b6fd210dced66e, a90e89fb031b0b357305ac9d874968d8451b1181 wpt-pr: 19993 UltraBlame original commit: 9b458e91114235eb43711dbac7a3acfe30c83fce

…ndition`, a=testonly Automatic update from web-platform-tests [testharness.js] introduce `assert_precondition` This depends on mozlog 5.0 for the new PRECONDITION_FAILED status: https://bugzilla.mozilla.org/show_bug.cgi?id=1589056 Implements web-platform-tests/rfcs#16. Includes parts of web-platform-tests/wpt#16689. Fixes web-platform-tests/wpt#19844. -- [vibration] use `assert_precondition` to avoid spurious pass This demonstrates the use of `assert_precondition` for subtests. -- wpt-commits: 17543bc30bdf65838e9ec8e3b3b6fd210dced66e, a90e89fb031b0b357305ac9d874968d8451b1181 wpt-pr: 19993

wpt-pr-bot added infra testharness.js WebCryptoAPI wg-webcrypto labels May 6, 2019

wpt-pr-bot assigned jgraham May 6, 2019

wpt-pr-bot requested review from gsnedders, jgraham and jimsch May 6, 2019 17:22

jimsch reviewed May 6, 2019

View reviewed changes

resources/testharness.js Show resolved Hide resolved

jimsch reviewed May 8, 2019

View reviewed changes

WebCryptoAPI/generateKey/successes.js Outdated Show resolved Hide resolved

jimsch marked this pull request as ready for review May 8, 2019 20:31

Hexcles force-pushed the skip branch from 25ddb0f to 689de1a Compare June 7, 2019 21:58

wpt-pr-bot added the wptrunner The automated test runner, commonly called through ./wpt run label Jun 7, 2019

wpt-pr-bot requested a review from jugglinmike June 7, 2019 21:58

Hexcles force-pushed the skip branch from 689de1a to 6ea92ca Compare June 7, 2019 22:01

lukebjerring mentioned this pull request Jul 16, 2019

RFC #16 - PRECONDITION_FAILED subtest result (status) web-platform-tests/rfcs#16

Merged

lukebjerring added the idlharness.js label Jul 25, 2019

lukebjerring force-pushed the skip branch from 6ea92ca to 4b1029a Compare August 15, 2019 20:23

lukebjerring changed the title ~~Add test.skip helper and SKIP subtest status~~ Add test.skip helper and PRECONDITION_FAILED subtest status Aug 15, 2019

lukebjerring force-pushed the skip branch from 4b1029a to 61b8566 Compare August 28, 2019 17:59

wpt-pr-bot added the speech-api label Aug 28, 2019

wpt-pr-bot requested a review from foolip August 28, 2019 17:59

lukebjerring changed the title ~~Add test.skip helper and PRECONDITION_FAILED subtest status~~ Add assert_precondition helper and PRECONDITION_FAILED subtest status Aug 28, 2019

lukebjerring force-pushed the skip branch from 61b8566 to 018a128 Compare August 28, 2019 19:35

Luke Bjerring and others added 2 commits September 19, 2019 16:25

Cleanup

cc2d212

Update tox.ini

358e866

foolip reviewed Sep 19, 2019

View reviewed changes

lukebjerring commented Oct 4, 2019

View reviewed changes

Fix declaration order

3304cfb

foolip mentioned this pull request Oct 23, 2019

Implement assert_precondition (RFC) #19844

Closed

foolip closed this Oct 23, 2019

foolip deleted the skip branch October 25, 2019 12:53

foolip mentioned this pull request Oct 30, 2019

[testharness.js] introduce assert_precondition #19993

Merged

Conversation

lukebjerring commented May 6, 2019

Uh oh!

Uh oh!

Uh oh!

jimsch commented May 8, 2019

Uh oh!

jgraham commented May 13, 2019

Uh oh!

Hexcles commented Jun 7, 2019

Uh oh!

jgraham commented Jul 16, 2019

Uh oh!

lukebjerring commented Aug 15, 2019

Uh oh!

lukebjerring commented Aug 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

foolip Sep 19, 2019

Choose a reason for hiding this comment

Uh oh!

lukebjerring Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

lukebjerring Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

foolip Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

lukebjerring Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

foolip Oct 5, 2019

Choose a reason for hiding this comment

Uh oh!

lukebjerring Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

lukebjerring commented Oct 10, 2019

Uh oh!

foolip commented Oct 10, 2019

Uh oh!

jgraham commented Oct 16, 2019

Uh oh!

foolip commented Oct 16, 2019

Uh oh!

foolip commented Oct 17, 2019

Uh oh!

foolip commented Oct 23, 2019

Uh oh!

foolip commented Nov 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lukebjerring commented Aug 30, 2019 •

edited

Loading