Repository navigation
Timeout browser suite loads - #2080
Merged
Merged
Conversation
The current timeout applies to starting the process and connecting back from the host. Add a timeout which applies on each suite as it is loaded. Change the chrome argument to log to `stderr` instead of `stdout`. The latter seems to have no effect. Eagerly decode and split lines from the browser output instead of storing the bytes to avoid mixing lines between the two outputs. In most cases this shouldn't matter, I have not seen output on `stdout`. Use `--v=0` instead if `--v=1` since the 0 level includes console output, and the extra browser level output with higher verbosity does not look useful.
jakemac53
approved these changes
Aug 11, 2023
Member
Author
|
@jakemac53 - I switched this from configurable per suite to hardcoded at 30 seconds. Do you have any concerns about that? This timeout does include waiting for The reason I hardcoded it is because we have some tests where the configured timeout seems to be too low for a suite and the load was timing out. |
Member
Author
|
I think it may work to leave it configurable so there is an escape hatch for slow |
Contributor
|
A lower bound but configurable is probably fine. |
natebosch
added a commit
that referenced
this pull request
Oct 12, 2026
Fixes #2691 The browser platform gave up waiting for a browser to load a compiled suite after the test timeout, or 30 seconds if that was longer. This timeout was added in #2080, before `suite_load_timeout` existed, to get better diagnostics for browser loads that never completed. The test timeout has nothing to do with loading, and `suite_load_timeout` had no effect on this limit, so the only way to give a slow browser more time was to raise the timeout for every test. Start a stopwatch in the load suite's body, right after the invoker starts the load test's timeout, and expose the time left before that timeout through a static `PlatformPlugin.remainingLoadTime` getter that platforms can read during `load`. It reads a zone value set by the load suite, so platforms need no new imports, and it is static so classes implementing `PlatformPlugin` don't need to change. It is `null` when loading can't time out because the suite load timeout is `none` or timeouts are ignored. The value is only meaningful during `load`; callbacks registered while loading run in the same zone and may see a stale value later. Have the browser platform give up 1 second before the remaining load time runs out, so its error, which includes the browser output, wins over the generic timeout on the load test. Retries in the loader share the same budget. When there is no remaining load time, don't time out the browser, matching how other platforms load suites without a suite load timeout. Configuring `suite_load_timeout` restores a limit along with the more detailed error. Bump test_core to 0.6.22-wip for the new API and pin it from test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current timeout applies to starting the process and connecting back
from the host. Add a timeout which applies on each suite as it is
loaded.
Change the chrome argument to log to
stderrinstead ofstdout. Thelatter seems to have no effect.
Eagerly decode and split lines from the browser output instead of
storing the bytes to avoid mixing lines between the two outputs. In most
cases this shouldn't matter, I have not seen output on
stdout.Use
--v=0instead of--v=1since the 0 level includes consoleoutput, and the extra browser level output with higher verbosity does
not look useful.