start and stop chromedriver once per sub-shard; do not wait for it to quit #69443
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| } | ||
| print('Stopping chromedriver'); | ||
| _chromeDriver.process.kill(); | ||
| while (await _isChromeDriverRunning()) { |
There was a problem hiding this comment.
My assumption is that pinging the chromedriver while it's shutting down is what's triggering the issue in the client socket implementation, so removing the need to wait for chromedriver to shut down should also 🤞 fix the flakiness.
| ].map(_withChromeDriver).toList(); | ||
| ]; | ||
| await _ensureChromeDriverIsRunning(); | ||
| await _selectIndexedSubshard(tests, kWebLongRunningTestShardCount); |
There was a problem hiding this comment.
Suggestion:Shall we add a comment explaining why we shutdown?
There was a problem hiding this comment.
I'm not sure? All our tests try to shutdown processes they start. It's like tearDown. If you can think of something that would explain something special about chromedriver, I'd be happy to add it. LMK.
Description
Start one shared instance of chromedriver and quit it at the end. This removes the need to check for driver connection at the end of the test, which seems to be leaving sockets that are not fully closed.
Related Issues
Fixes #69421