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

Add RFC for crash test type #33

Merged
merged 2 commits into from Nov 11, 2019
Merged

Add RFC for crash test type #33

merged 2 commits into from Nov 11, 2019

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Oct 9, 2019

Add a test type that just ensures that a page can be loaded without error.

@jgraham
Copy link
Contributor Author

jgraham commented Oct 9, 2019

@emilio I think you were interested in this. If you have feedback it would be great, particularly if you have evidence that sharing load-type tests (e.g. gecko crashtests) between engines is effective at finding issues.

Copy link
Member

@zcorpan zcorpan left a comment

Choose a reason for hiding this comment

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

wpt already has a bunch of crash tests, that might use testharness.js or reftest just to be able to run. e.g.
css/css-overflow/shrink-to-fit-auto-overflow-relayout-crash.html
css/css-pseudo/first-letter-of-html-root-crash.html

rfcs/load_test.md Outdated Show resolved Hide resolved
rfcs/load_test.md Outdated Show resolved Hide resolved
@jugglinmike
Copy link
Contributor

Crashes and other issues caught by these tests may not be sufficiently common between browsers to make sharing these tests a good use of resources.

This was my concern while reading the proposal, so I'm glad you included it! Personally, I'm pessimistic that tests like these do provide much value between implementations. I'm no implementer, though, so I'm very curious about what @emilio has to say on the subject.

@emilio
Copy link

emilio commented Oct 12, 2019

And this was a very simple test runner, there's tons of test-cases that I haven't ran or what not.

@gsnedders
Copy link
Member

I'd rather call them crash tests, given that's what they're called everywhere else.

@jgraham
Copy link
Contributor Author

jgraham commented Oct 14, 2019

The reason for not calingthem crashtests was that crashing is only one possible failure mode; they could also fail with an assert or sanitizer failure or similar in vendor infrastructure. But I'm sympathetic to the idea that "load" can be confusing and there is precedent here.

@gsnedders
Copy link
Member

I think that's an overly pedantic viewpoint to take when there's prior art in all vendors here already, even if the name is slightly confusing. (Just make all assertions and saniziter failures crashes, pff!)

rfcs/load_test.md Show resolved Hide resolved
rfcs/load_test.md Show resolved Hide resolved
rfcs/load_test.md Outdated Show resolved Hide resolved
rfcs/load_test.md Show resolved Hide resolved
@jgraham
Copy link
Contributor Author

jgraham commented Oct 31, 2019

web-platform-tests/wpt#20017 is a draft PR for this RFC. It's not done yet (hence draft), but what's there seems to work.

Copy link
Contributor Author

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

I think the only substantive decision left here is whether to use -crash or -crashtest in the filename. The former requires some additional work to rename existing files, but is also shorter and closer to existing practice for WebKit. I'd like to make a decision there and then close out this issue so we can land the changes.

@foolip
Copy link
Member

foolip commented Nov 5, 2019

My preference is for -crash.

@jgraham jgraham changed the title Add RFC for load test type Add RFC for crash test type Nov 11, 2019
@jgraham
Copy link
Contributor Author

jgraham commented Nov 11, 2019

OK changed to -crash and crashtests. So I don't think there are more remaining issues and will merge.

@jgraham jgraham merged commit 4940834 into master Nov 11, 2019
@foolip foolip deleted the load_test branch November 12, 2019 12:50
@Hexcles
Copy link
Member

Hexcles commented Apr 7, 2020

Doing some archeology here, I wonder when we changed the wait class from wait to test-wait? @jgraham

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants