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.

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

[testharness.js] introduce assert_precondition #19993

Merged
merged 2 commits into from Nov 8, 2019

Conversation

foolip
Copy link
Member

@foolip foolip commented Oct 30, 2019

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 foolip force-pushed the foolip/assert_precondition branch 3 times, most recently from 4fdb848 to 3e9e171 Compare November 2, 2019 00:24
@wpt-pr-bot wpt-pr-bot added infra testharness.js wptrunner The automated test runner, commonly called through ./wpt run labels Nov 2, 2019
@foolip foolip marked this pull request as ready for review November 4, 2019 12:58
@wpt-pr-bot wpt-pr-bot added the docs label Nov 4, 2019
@foolip
Copy link
Member Author

foolip commented Nov 4, 2019

https://wpt.fyi/results/vibration/invalid-values.html?diff&filter=ADC&run_id=344870003&run_id=344860005 shows that this had the intended effect for one test at least.

Copy link
Member Author

@foolip foolip left a comment

Choose a reason for hiding this comment

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

Bringing a detail to the attention of RFC reviewers coming from web-platform-tests/rfcs#16 (comment).

@@ -3671,16 +3692,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)

@@ -792,8 +792,10 @@ policies and contribution forms [3].

function done() {
if (tests.tests.length === 0) {
tests.status.status = tests.status.ERROR;
tests.status.message = "done() was called without first defining any tests";
if (tests.status.status === null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This extra check is a workaround for #20036 (comment) but it might change, I'll use whatever change @jugglinmike lands to fix it.

resources/testharness.js Outdated Show resolved Hide resolved
@foolip
Copy link
Member Author

foolip commented Nov 8, 2019

With #20168 now merged I'll rebase this.

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.
This demonstrates the use of `assert_precondition` for subtests.
@foolip
Copy link
Member Author

foolip commented Nov 8, 2019

Alright, given no concerns voices about the behavior of a bare assert_precondition outside of setup or a test, I'll go ahead and merge this. As we change tests to use this we might discover that this wasn't quite right and can change it.

@foolip foolip merged commit a90e89f into master Nov 8, 2019
@foolip foolip deleted the foolip/assert_precondition branch November 8, 2019 12:48
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 16, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 16, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 16, 2019
Address upstream WPT test harness changes

web-platform-tests/wpt#19449 and web-platform-tests/wpt#19993 caused some of our proprietary tests to silently start to fail. This addresses all of the breakage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs infra testharness.js vibration wg-das wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement assert_precondition (RFC)
5 participants