Repository navigation
[tool] Let the OS pick a port for chromedriver and wait for exit - #13355
Conversation
| final stdoutBuffer = StringBuffer(); | ||
| final stderrBuffer = StringBuffer(); | ||
| final portPattern = RegExp(r'started successfully on port (\d+)'); | ||
| chromedriver.stderr |
There was a problem hiding this comment.
Should we be cancelling these subscriptions when we're done?
There was a problem hiding this comment.
I moved the process and both subscriptions into a _ChromeDriver class whose stop() cancels them after await exitCode in 17100d3d98d. They can't be cancelled as soon as we get the port because Process.start requires draining stdout/stderr while chromedriver is still running. Output is no longer buffered once the port is known.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors DriveExamplesCommand to dynamically allocate the ChromeDriver port by starting it on port 0 and parsing the assigned port from stdout, and adds corresponding tests. Feedback indicates that the regular expression used to parse the port does not match actual ChromeDriver output, which would cause the tool to hang in production. Additionally, suggestions are made to prevent unbounded memory growth in the output buffers and to update the test mocks to use realistic ChromeDriver output.
mdebbar
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/packages/13355, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter/packages@3f34a6f...1df8447 2026-11-03 [email protected] [webview_flutter] Convert iOS impl example app to Swift (flutter/packages#13538) 2026-11-03 [email protected] [material_ui] Temporarily exclude material_ui from non-Wasm web unit tests (flutter/packages#13537) 2026-11-02 [email protected] [material_ui] Removes list semantics in some a11y_assessment samples (flutter/packages#13257) 2026-11-02 [email protected] [material_ui] Use named uniforms instead of setFloat in InkSparkle (flutter/packages#13445) 2026-11-02 [email protected] [VPAT] Fix contrast and reflow in three a11y_assessments demos (flutter/packages#13409) 2026-11-02 [email protected] [two_dimensional_scrollables] Fix pinned cells hit test (flutter/packages#13376) 2026-11-02 [email protected] [tool] Let the OS pick a port for chromedriver and wait for exit (flutter/packages#13355) 2026-11-02 [email protected] [vector_graphics] Draw <text> under an RTL ambient direction (flutter/packages#13408) 2026-11-02 [email protected] [VPAT] Keep a11y_assessments snackbars open until closed (flutter/packages#13451) 2026-11-02 [email protected] [pigeon] Fix race in event channel handler removal integration test (flutter/packages#13528) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Part of flutter/flutter#194313
DriveExamplesCommandstartschromedriver --port=4444inside the per-example loop without waiting for it to bind, and stops it withchromedriver.kill()without awaitingchromedriver.exitCode. When a package has multiple web examples (e.g.google_maps_flutter_web'sexample/latestandexample/3-64), the dyingchromedriverfrom the first example still holds port4444when the next example spawns a newchromedriver --port=4444, so the secondchromedriverfails to bind and exits immediately:Example CI failures:
This PR:
chromedriver --port=0and waits forstarted successfully on port (\d+)on stdout before runningflutter drive --driver-port=<port>.chromedriver.exitCodeafterchromedriver.kill()in afinallyblock.