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

Also ignore ERROR in slow test check #13158

Merged
merged 1 commit into from
Sep 24, 2018
Merged

Conversation

Hexcles
Copy link
Member

@Hexcles Hexcles commented Sep 21, 2018

Tweak the docstring accordingly to explain why.

@jgraham this is the bug you pointed out to me earlier. PTAL, especially to see if my docstring makes sense.

Tweak the docstring accordingly to explain why.
@wpt-pr-bot wpt-pr-bot added infra wptrunner The automated test runner, commonly called through ./wpt run labels Sep 21, 2018
@jgraham
Copy link
Contributor

jgraham commented Sep 24, 2018

The other option I had in mind was to check if the recorded time was greater than the expected timeout, and not fail the check if it was. But let's see if this works well first.

@jgraham jgraham merged commit 1306690 into master Sep 24, 2018
@Hexcles
Copy link
Member Author

Hexcles commented Sep 24, 2018

@jgraham I thought about that, too, but decided to go with this way because of the reasons explained in the docstring. Even if the run time of a crashy test fluctuates around the timeout, as long as the test consistently crashes, it won't be flaky as the result is always CRASH. Also, this is a more conservative approach in general so there'll less likely be false positives.

@Hexcles Hexcles deleted the ignore-error-in-slow-tests branch September 24, 2018 14:29
@foolip
Copy link
Member

foolip commented Sep 24, 2018

This was follow up to #11570.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants