Make the test suite load timeout configurable #390
Labels
Comments
|
+100 |
|
+1 |
1 similar comment
|
👍 |
|
@greglittlefield-wf Does the @timeout annotation not work here? |
|
The timeout used when waiting for a browser to start is different than the timeout for any given test, in part because a single browser instance is used across many tests. |
|
+1 |
Merged
|
@grouma - if this was configurable would it be useful for us internally? |
|
Initially it would have been useful but since I extended the timeout we have not run into any issues. It may still be worth exposing if others are running into issues. |
|
Is anyone still seeing timeouts here? Adding extra configuration is a real cost, so I'd like to just find a good default here rather than making users set it. |
|
Yes, we're still seeing timeouts for larger suites in content_shell/Dartium, as well as timeouts for not-as-large suites when run in the Dart Dev Compiler. |
|
How are you running test with DDC? |
|
Something like: Serve via: pub serve test --port 8081 --web-compiler dartdevcRun tests via: pub run test -p chrome --pub-serve 8081 |
|
@jakemac53 is it reasonable to expect that a test suite might take upwards of 8 minutes to compile? |
|
We have real examples internally where just dart2js takes that long, so ya. Externally with ddc that shouldn't be common, but it wouldn't be inconceivable for large projects. I know of projects that aren't that large which take over a minute for instance. I don't think it is really safe to assume any sort of maximum amount of time a build might take. In general I prefer the --precompiled approach to the --pub-serve approach partially for this reason. |
|
We could just disable the timeout entirely. Eight minutes is already to the point where it's not going to time out quickly enough to be especially useful in any workflow involving human interaction. |
|
Below is the actual timeout error I'm seeing when running a larger suite in the Dev Compiler, if that helps. Interestingly, the error says "Test timed out after 5 minutes", not 8 minutes... Run using:
pub run test --concurrency=1 -p chrome --reporter=expanded --pub-serve=8081 test/ui_components_1_test.dart | ts '[%Y-%m-%d %H:%M:%S]' |
|
This timeout was introduced to give us details when a DDC test would fail on Forge and result in a test hang. In that case we would not get any information. I believe that no longer can happen since I introduced this change: #682 We can conceivably remove this timeout. |
|
That error trace is interesting for a couple of reasons:
This suggests that the deserialize timeout is already redundant, but there's a bug in how we're handling the load suite timeout that makes it not actually shut down the test. We should fix that bug and make the load suite timeout long enough to avoid killing compilation while also not hanging Forge—maybe 15m? |
nex3
added a commit
that referenced
this issue
Nov 7, 2017
We weren't completing the _suiteAndZone completer, which meant that errors (such as timeouts) were reported but didn't actually stop the suite from loading. See #390
nex3
added a commit
that referenced
this issue
Nov 7, 2017
This can never fire before the load suite timeout. Now that that timeout actually works, it's redundant. See #390
nex3
added a commit
that referenced
this issue
Nov 7, 2017
nex3
added a commit
that referenced
this issue
Nov 8, 2017
This can never fire before the load suite timeout. Now that that timeout actually works, it's redundant. See #390
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We're currently getting intermittent test suite timeouts in our CI environment, since our suite loading time is hovering around 1 minute, even with concurrency=1.
It'd be awesome if that 1 minute suite timeout could be configurable.
The text was updated successfully, but these errors were encountered: