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

Add note to crashtest.md about testharness.js #22401

Merged
merged 1 commit into from Mar 23, 2020
Merged

Add note to crashtest.md about testharness.js #22401

merged 1 commit into from Mar 23, 2020

Conversation

stephenmcgruer
Copy link
Contributor

Authors often assume that they need to include testharness.js or use the
standard 'test(..)' methods they are familiar with when writing crash
tests, which is not the case.

@stephenmcgruer
Add note to crashtest.md about testharness.js
16bdeea 
Authors often assume that they need to include testharness.js or use the
standard 'test(..)' methods they are familiar with when writing crash
tests, which is not the case.
jgraham
jgraham approved these changes Mar 23, 2020
View reviewed changes
Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

We should probably lint for this.

@stephenmcgruer
Copy link
Contributor Author

We should probably lint for this.

Agreed, will land this and start on the lint.

@jgraham jgraham merged commit 4453287 into master Mar 23, 2020
@jgraham jgraham deleted the smcgruer/crashd-ocs branch March 23, 2020 19:52
@stephenmcgruer
Copy link
Contributor Author

Hah, we already have one.

$ cp ./dom/svg-insert-crash.html ./dom/svg-insert-new-crash.html
$ ./wpt lint ./dom/svg-insert-new-crash.html 
ERROR:lint:dom/svg-insert-new-crash.html: testharness.js included in a crashtest test (TESTHARNESS-IN-OTHER-TYPE)
INFO:lint:
INFO:lint:There was 1 error (TESTHARNESS-IN-OTHER-TYPE: 1)
INFO:lint:You must fix all errors; for details on how to fix them, see
INFO:lint:https://web-platform-tests.org/writing-tests/lint-tool.html
INFO:lint:
INFO:lint:However, instead of fixing a particular error, it's sometimes
INFO:lint:OK to add a line to the lint.whitelist file in the root of the
INFO:lint:web-platform-tests directory to make the lint tool ignore it.
INFO:lint:
INFO:lint:For example, to make the lint tool ignore all 'TESTHARNESS-IN-OTHER-TYPE'
INFO:lint:errors in the dom/svg-insert-new-crash.html file,
INFO:lint:you could add the following line to the lint.whitelist file.
INFO:lint:
INFO:lint:TESTHARNESS-IN-OTHER-TYPE: dom/svg-insert-new-crash.html

Weirdly I did discover that calling something foo-crash-new.html doesn't have it detected as a crashtest, which seems to contradict the docs (https://web-platform-tests.org/writing-tests/crashtest.html just says 'identified by the string -crash in the filename').

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

@jgraham jgraham jgraham approved these changes

@sideshowbarker sideshowbarker Awaiting requested review from sideshowbarker

Assignees

@sideshowbarker sideshowbarker

Labels
docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stephenmcgruer @jgraham @sideshowbarker @wpt-pr-bot