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

Jump to bottom

[testharness.js] Preserve harness error message #20168

Merged
merged 1 commit into from Nov 8, 2019
Merged

[testharness.js] Preserve harness error message #20168

merged 1 commit into from Nov 8, 2019

Conversation

jugglinmike
Copy link
Contributor

This change does not modify tests because it only effects the harness status string, and we do not validate that.

I checked the "raw" wpt.fyi results for gh-20036. There were 10 occurrences of the generic harness error message in Chrome's data:

$ grep 'called without first defining' report-master-chrome-pretty.json -C 3 | grep 'test":'
"test": "/idle-detection/interceptor.https.html",
"test": "/fetch/metadata/sec-fetch-dest/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub.html",
"test": "/fetch/metadata/sec-fetch-dest/redirect/redirect-http-upgrade.tentative.sub.html",
"test": "/fetch/metadata/sec-fetch-dest/redirect/redirect-https-downgrade.tentative.sub.html",
"test": "/streams/readable-byte-streams/construct-byob-request.any.sharedworker.html",
"test": "/streams/readable-byte-streams/construct-byob-request.any.serviceworker.html",
"test": "/streams/readable-byte-streams/construct-byob-request.any.html",
"test": "/streams/readable-byte-streams/construct-byob-request.any.worker.html",
"test": "/infrastructure/expected-fail/uncaught-exception.html",
"test": "/infrastructure/expected-fail/unhandled-rejection.html",

Firefox had a significantly greater number:

$ grep 'called without first defining' report-master-firefox-pretty.json | wc -l
346

Following the application of this patch, both browsers report just one test--a legitimate and previously-unreported testing error:

$ grep 'called without first defining' report-patched-chrome-pretty.json -C 3 | grep 'test":'
"test": "/css/css-lists/list-inside-contain.html",
$ grep 'called without first defining' report-patched-firefox-pretty.json -C 3 | grep 'test":'
"test": "/css/css-lists/list-inside-contain.html",

That test was merged after gh-20036, so we shouldn't lose sleep about any lack of care. It was most likely submitted from a branch that didn't include the single_test enforcement. In any event, I've opened gh-20167 to correct it.

Reviewing this patch's effect on results more generally, it doesn't appear to have any unintended side effects:

Although many of the discrepancies are due to pre-existing instabilities, I haven't yet reviewed all of them. I'm opening this pull request now because this problem is blocking gh-19993, so others can verify in my place if they want to move forward before I return tomorrow (EST).

foolip
foolip approved these changes Nov 8, 2019
View reviewed changes
Copy link
Member

@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.

Thanks for the thorough description and finding that outlier test!

@foolip foolip merged commit 84fdc88 into web-platform-tests:master Nov 8, 2019
@foolip foolip mentioned this pull request Nov 8, 2019
Merged
@jugglinmike
Copy link
Contributor Author

You bet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@foolip foolip foolip approved these changes

@gsnedders gsnedders Awaiting requested review from gsnedders

@jgraham jgraham Awaiting requested review from jgraham

Assignees

@gsnedders gsnedders

Labels
infra testharness.js
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jugglinmike @foolip @gsnedders @wpt-pr-bot @moz-wptsync-bot