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.

Sign up for GitHub

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

Jump to bottom

[testharness.js] introduce assert_precondition #19993

Merged
merged 2 commits into from Nov 8, 2019
Merged

[testharness.js] introduce assert_precondition #19993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
962c338
[testharness.js] introduce `assert_precondition`
foolip Oct 25, 2019
f378614
[vibration] use `assert_precondition` to avoid spurious pass
foolip Nov 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 45 additions & 3 deletions docs/writing-tests/testharness-api.md
View file
Open in desktop
Expand Up @@ -305,6 +305,41 @@ NOTE: All asserts must be located in a `test()` or a step of an
these places won't be detected correctly by the harness and may cause
unexpected exceptions that will lead to an error in the harness.

## Preconditions ##

When a test would be invalid unless certain conditions are met, but yet
doesn't explicitly depend on those preconditions, `assert_precondition` can be
used. For example:

```js
async_test((t) => {
const video = document.createElement("video");
assert_precondition(video.canPlayType("video/webm"));
video.src = "multitrack.webm";
// test something specific to multiple audio tracks in a WebM container
t.done();
}, "WebM with multiple audio tracks");
```

A failing `assert_precondition` call is reported as a status of
`PRECONDITION_FAILED` for the subtest.

`assert_precondition` can also be used during test setup. For example:

```js
setup(() => {
assert_precondition("onfoo" in document.body, "'foo' event supported");
});
async_test(() => { /* test #1 waiting for "foo" event */ });
async_test(() => { /* test #2 waiting for "foo" event */ });
```

A failing `assert_precondition` during setup is reported as a status of
`PRECONDITION_FAILED` for the test, and the subtests will not run.

See also the `.optional` [file name convention](file-names.md), which is
appropriate when the precondition is explicitly optional behavior.

## Cleanup ##

Occasionally tests may create state that will persist beyond the test itself.
Expand Down Expand Up @@ -521,17 +556,20 @@ the following methods:
Tests have the following properties:

* `status` - A status code. This can be compared to the `PASS`, `FAIL`,
`TIMEOUT` and `NOTRUN` properties on the test object
`PRECONDITION_FAILED`, `TIMEOUT` and `NOTRUN` properties on the
test object

* `message` - A message indicating the reason for failure. In the future this
will always be a string

The status object gives the overall status of the harness. It has the
following properties:

* `status` - Can be compared to the `OK`, `ERROR` and `TIMEOUT` properties
* `status` - Can be compared to the `OK`, `PRECONDITION_FAILED`, `ERROR` and
`TIMEOUT` properties

* `message` - An error message set when the status is `ERROR`
* `message` - An error message set when the status is `PRECONDITION_FAILED`
or `ERROR`.

## External API ##

Expand Down Expand Up @@ -706,6 +744,10 @@ workers and want to ensure they run in series:

## List of Assertions ##

### `assert_precondition(condition, description)`
asserts that `condition` is truthy.
See [preconditions](#preconditions) for usage.

### `assert_true(actual, description)`
asserts that `actual` is strictly true

Expand Down
10 changes: 10 additions & 0 deletions infrastructure/expected-fail/precondition-in-promise.html
View file
Open in desktop
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<meta charset=utf-8>
<title>Precondition in promise</title>
foolip marked this conversation as resolved.
Show resolved Hide resolved
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
new Promise(() => {
assert_precondition(false);
});
</script>
10 changes: 10 additions & 0 deletions infrastructure/expected-fail/precondition-in-setup.html
View file
Open in desktop
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<meta charset=utf-8>
<title>Precondition in setup</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
setup(() => {
assert_precondition(false);
});
</script>
8 changes: 8 additions & 0 deletions infrastructure/expected-fail/precondition-without-setup.html
View file
Open in desktop
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<meta charset=utf-8>
<title>Precondition without wrapping setup</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
assert_precondition(false);
</script>
31 changes: 31 additions & 0 deletions infrastructure/expected-fail/precondition.html
View file
Open in desktop
@@ -0,0 +1,31 @@
<!DOCTYPE html>
<meta charset=utf-8>
<title>Preconditions in tests</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(() => {
assert_precondition(false, 'precondition 1');
}, 'test');

async_test((t) => {
assert_precondition(false, 'precondition 2');
t.done();
}, 'async_test immediate');

async_test((t) => {
t.step_timeout(() => {
assert_precondition(false, 'precondition 3');
t.done();
}, 100);
}, 'async_test after timeout');

promise_test(async () => {
assert_precondition(false, 'precondition 4');
}, 'promise_test immediate');

promise_test(async () => {
await Promise.resolve();
assert_precondition(false, 'precondition 5');
}, 'promise_test after await');
</script>
2 changes: 2 additions & 0 deletions infrastructure/metadata/infrastructure/expected-fail/precondition-in-promise.html.ini
View file
Open in desktop
@@ -0,0 +1,2 @@
[precondition-in-promise.html]
expected: PRECONDITION_FAILED
2 changes: 2 additions & 0 deletions infrastructure/metadata/infrastructure/expected-fail/precondition-in-setup.html.ini
View file
Open in desktop
@@ -0,0 +1,2 @@
[precondition-in-setup.html]
expected: PRECONDITION_FAILED
2 changes: 2 additions & 0 deletions infrastructure/metadata/infrastructure/expected-fail/precondition-without-setup.html.ini
View file
Open in desktop
@@ -0,0 +1,2 @@
[precondition-without-setup.html]
expected: PRECONDITION_FAILED
15 changes: 15 additions & 0 deletions infrastructure/metadata/infrastructure/expected-fail/precondition.html.ini
View file
Open in desktop
@@ -0,0 +1,15 @@
[precondition.html]
[test]
expected: PRECONDITION_FAILED

[async_test immediate]
expected: PRECONDITION_FAILED

[async_test after timeout]
expected: PRECONDITION_FAILED

[promise_test immediate]
expected: PRECONDITION_FAILED

[promise_test after await]
expected: PRECONDITION_FAILED
48 changes: 37 additions & 11 deletions resources/testharness.js
View file
Open in desktop
Expand Up @@ -1821,6 +1821,13 @@ policies and contribution forms [3].
}
expose(assert_any, "assert_any");

function assert_precondition(precondition, description) {
if (!precondition) {
throw new PreconditionFailedError(description);
}
}
expose(assert_precondition, "assert_precondition");

function Test(name, properties)
{
if (tests.file_is_test && tests.tests.length) {
Expand Down Expand Up @@ -1865,7 +1872,8 @@ policies and contribution forms [3].
PASS:0,
FAIL:1,
TIMEOUT:2,
NOTRUN:3
NOTRUN:3,
PRECONDITION_FAILED:4
};

Test.prototype = merge({}, Test.statuses);
Expand Down Expand Up @@ -1925,10 +1933,11 @@ policies and contribution forms [3].
if (this.phase >= this.phases.HAS_RESULT) {
return;
}
var status = e instanceof PreconditionFailedError ? this.PRECONDITION_FAILED : this.FAIL;
var message = String((typeof e === "object" && e !== null) ? e.message : e);
var stack = e.stack ? e.stack : null;

this.set_status(this.FAIL, message, stack);
this.set_status(status, message, stack);
this.phase = this.phases.HAS_RESULT;
this.done();
}
Expand Down Expand Up @@ -2399,7 +2408,8 @@ policies and contribution forms [3].
TestsStatus.statuses = {
OK:0,
ERROR:1,
TIMEOUT:2
TIMEOUT:2,
PRECONDITION_FAILED:3
};

TestsStatus.prototype = merge({}, TestsStatus.statuses);
Expand Down Expand Up @@ -2505,7 +2515,7 @@ policies and contribution forms [3].
try {
func();
} catch (e) {
this.status.status = this.status.ERROR;
this.status.status = e instanceof PreconditionFailedError ? this.status.PRECONDITION_FAILED : this.status.ERROR;
this.status.message = String(e);
this.status.stack = e.stack ? e.stack : null;
this.complete();
Expand Down Expand Up @@ -3063,12 +3073,14 @@ policies and contribution forms [3].
status_text_harness[harness_status.OK] = "OK";
status_text_harness[harness_status.ERROR] = "Error";
status_text_harness[harness_status.TIMEOUT] = "Timeout";
status_text_harness[harness_status.PRECONDITION_FAILED] = "Precondition Failed";

var status_text = {};
status_text[Test.prototype.PASS] = "Pass";
status_text[Test.prototype.FAIL] = "Fail";
status_text[Test.prototype.TIMEOUT] = "Timeout";
status_text[Test.prototype.NOTRUN] = "Not Run";
status_text[Test.prototype.PRECONDITION_FAILED] = "Precondition Failed";

var status_number = {};
forEach(tests,
Expand Down Expand Up @@ -3450,6 +3462,13 @@ policies and contribution forms [3].
return lines.slice(i).join("\n");
}

function PreconditionFailedError(message)
{
AssertionError.call(this, message);
}
PreconditionFailedError.prototype = Object.create(AssertionError.prototype);
expose(PreconditionFailedError, "PreconditionFailedError");

function make_message(function_name, description, error, substitutions)
{
for (var p in substitutions) {
Expand Down Expand Up @@ -3677,16 +3696,19 @@ policies and contribution forms [3].
var tests = new Tests();

if (global_scope.addEventListener) {
var error_handler = function(message, stack) {
var error_handler = function(error, message, stack) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be reasonable disagreement to be had about this one, whether an unwrapped precondition_failed should be an error or not. And also whether it really makes sense for allow_uncaught_exception to allow failing asserts as is currently the case.

web-platform-tests/rfcs#16 (comment)

var precondition_failed = error instanceof PreconditionFailedError;
if (tests.file_is_test) {
var test = tests.tests[0];
if (test.phase >= test.phases.HAS_RESULT) {
return;
}
test.set_status(test.FAIL, message, stack);
var status = precondition_failed ? test.PRECONDITION_FAILED : test.FAIL;
test.set_status(status, message, stack);
test.phase = test.phases.HAS_RESULT;
} else if (!tests.allow_uncaught_exception) {
tests.status.status = tests.status.ERROR;
var status = precondition_failed ? tests.status.PRECONDITION_FAILED : tests.status.ERROR;
tests.status.status = status;
tests.status.message = message;
tests.status.stack = stack;
}
Expand All @@ -3708,7 +3730,7 @@ policies and contribution forms [3].
} else {
stack = e.filename + ":" + e.lineno + ":" + e.colno;
}
error_handler(message, stack);
error_handler(e.error, message, stack);
}, false);

addEventListener("unhandledrejection", function(e) {
Expand All @@ -3722,7 +3744,7 @@ policies and contribution forms [3].
if (e.reason && e.reason.stack) {
stack = e.reason.stack;
}
error_handler(message, stack);
error_handler(e.reason, message, stack);
}, false);
}

Expand Down Expand Up @@ -3760,7 +3782,7 @@ table#results {\
\
table#results th:first-child,\
table#results td:first-child {\
width:4em;\
width:8em;\
}\
\
table#results th:last-child,\
Expand Down Expand Up @@ -3801,7 +3823,11 @@ tr.notrun > td:first-child {\
color:blue;\
}\
\
.pass > td:first-child, .fail > td:first-child, .timeout > td:first-child, .notrun > td:first-child {\
tr.preconditionfailed > td:first-child {\
color:blue;\
}\
\
.pass > td:first-child, .fail > td:first-child, .timeout > td:first-child, .notrun > td:first-child, .preconditionfailed > td:first-child {\
font-variant:small-caps;\
}\
\
Expand Down
6 changes: 4 additions & 2 deletions tools/wptrunner/wptrunner/executors/base.py
View file
Open in desktop
Expand Up @@ -58,12 +58,14 @@ def strip_server(url):
class TestharnessResultConverter(object):
harness_codes = {0: "OK",
1: "ERROR",
2: "TIMEOUT"}
2: "TIMEOUT",
3: "PRECONDITION_FAILED"}

test_codes = {0: "PASS",
1: "FAIL",
2: "TIMEOUT",
3: "NOTRUN"}
3: "NOTRUN",
4: "PRECONDITION_FAILED"}

def __call__(self, test, result, extra=None):
"""Convert a JSON result into a (TestResult, [SubtestResult]) tuple"""
Expand Down
4 changes: 2 additions & 2 deletions tools/wptrunner/wptrunner/wpttest.py
View file
Open in desktop
Expand Up @@ -47,12 +47,12 @@ def __repr__(self):

class TestharnessResult(Result):
default_expected = "OK"
statuses = {"OK", "ERROR", "INTERNAL-ERROR", "TIMEOUT", "EXTERNAL-TIMEOUT", "CRASH"}
statuses = {"OK", "ERROR", "INTERNAL-ERROR", "TIMEOUT", "EXTERNAL-TIMEOUT", "CRASH", "PRECONDITION_FAILED"}


class TestharnessSubtestResult(SubtestResult):
default_expected = "PASS"
statuses = {"PASS", "FAIL", "TIMEOUT", "NOTRUN"}
statuses = {"PASS", "FAIL", "TIMEOUT", "NOTRUN", "PRECONDITION_FAILED"}


class ReftestResult(Result):
Expand Down
1 change: 1 addition & 0 deletions vibration/invalid-values.html
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
<div id='log'></div>
<script>
test(function() {
assert_precondition(navigator.vibrate, 'navigator.vibrate exists');
assert_throws(new TypeError(), function() {
navigator.vibrate();
}, 'Argument is required, so was expecting a TypeError.');
Expand Down