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

[wptrunner] Cycle testdriver event loop when testharness isn't loaded #48582

Conversation

jonathan-j-lee
Copy link
Contributor

Workaround for https://crbug.com/340662810 that seems to work without an increase in test runtime or regressions (https://crrev.com/c/5698330/9). On Windows (the affected platform):

  • With patch: 225 min total shard runtime, 1873 regressions, 322 unexpected timeouts
  • Without patch: 225 min total shard runtime, 1888 regressions, 326 unexpected timeouts

Waiting for the load event doesn't seem to work (https://crrev.com/c/5698330/7).

Workaround for https://crbug.com/340662810 that seems to work
without an increase in test runtime or regressions
(https://crrev.com/c/5698330/9).
@jonathan-j-lee jonathan-j-lee marked this pull request as ready for review October 11, 2024 21:58
@wpt-pr-bot wpt-pr-bot added infra wptrunner The automated test runner, commonly called through ./wpt run labels Oct 11, 2024
Copy link
Contributor

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

This does not happen to a lot of tests, so should not have an impact on total test time.

@jonathan-j-lee
Copy link
Contributor Author

This does not happen to a lot of tests

I was running into a lot of failed flakiness checks, so hopefully this should mitigate that.

should not have an impact on total test time.

My worry was that testharnessreport.js would never load correctly, which would cause the executor to spin indefinitely. But as shown, that does not seem to be the case.

@jonathan-j-lee jonathan-j-lee merged commit 280d04e into web-platform-tests:master Oct 11, 2024
33 checks passed
@jonathan-j-lee jonathan-j-lee deleted the wptrunner/chrome/retry-testharness branch October 11, 2024 22:14
@WeizhongX
Copy link
Contributor

This does not happen to a lot of tests

I was running into a lot of failed flakiness checks, so hopefully this should mitigate that.

should not have an impact on total test time.

My worry was that testharnessreport.js would never load correctly, which would cause the executor to spin indefinitely. But as shown, that does not seem to be the case.

If this happens, will the test timeout?

@jonathan-j-lee
Copy link
Contributor Author

If this happens, will the test timeout?

Hypothetically, it would, I think, since even non-testdriver testharness tests use the message queue/testdriver loop to signal completion. The executor process would report EXTERNAL-TIMEOUT and need to be killed by its TestRunnerManager.

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