[dotnet][java][py][rb] pass --enable-chrome-logs unless CHROME_LOG_FILE is set - #17858
Conversation
PR Summary by QodoRespect CHROME_LOG_FILE and default --enable-chrome-logs across bindings
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1.
|
8ce198b to
c38c1df
Compare
|
Code review by qodo was updated up to the latest commit c38c1df |
There was a problem hiding this comment.
Pull request overview
This PR updates multiple Selenium bindings and CI tooling to reduce unwanted browser stdout/stderr leaking into user consoles by enabling Chrome/Edge browser logging capture in the driver logs. It standardizes passing --enable-chrome-logs by default while avoiding doing so when the user has explicitly configured browser logging via CHROME_LOG_FILE (and adds CI rerun verbosity to make debugging failures easier).
Changes:
- Enable
--enable-chrome-logsby default for Java and Ruby driver services (Chrome/Edge), aligning behavior across bindings. - Stop overriding user intent when
CHROME_LOG_FILEis set (Python/.NET fixes + Java/Ruby support). - Improve CI rerun diagnostics by rerunning failures with full Bazel test output and enabling rerun-with-debug for .NET CI jobs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/github-actions/rerun-failures.sh | Adds --test_output=all to reruns to improve failure diagnostics. |
| .github/workflows/ci-dotnet.yml | Enables rerun-with-debug for .NET Windows jobs to leverage rerun-failures debugging. |
| dotnet/src/webdriver/Chromium/ChromiumDriverService.cs | Avoids adding --enable-chrome-logs when CHROME_LOG_FILE is present. |
| py/selenium/webdriver/chrome/service.py | Skips --enable-chrome-logs when CHROME_LOG_FILE is set (but needs to respect the configured service env mapping). |
| py/selenium/webdriver/edge/service.py | Same as Chrome Python service (but needs to respect the configured service env mapping). |
| rb/lib/selenium/webdriver/chrome/service.rb | Adds default --enable-chrome-logs unless CHROME_LOG_FILE is set (but current code will crash when args is nil). |
| rb/lib/selenium/webdriver/edge/service.rb | Same as Chrome Ruby service (but current code will crash when args is nil). |
| rb/spec/unit/selenium/webdriver/chrome/service_spec.rb | Updates unit coverage to assert default enablement and non-duplication of --enable-chrome-logs. |
| rb/spec/unit/selenium/webdriver/edge/service_spec.rb | Updates unit coverage to assert default enablement and non-duplication of --enable-chrome-logs. |
| rb/spec/unit/selenium/webdriver/common/service_spec.rb | Updates shortcut constructors to include --enable-chrome-logs for Chrome/Edge services. |
| java/src/org/openqa/selenium/chrome/ChromeDriverService.java | Adds --enable-chrome-logs conditionally (non-Windows and only when CHROME_LOG_FILE is not set). |
| java/src/org/openqa/selenium/edge/EdgeDriverService.java | Same as Chrome Java service. |
| java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java | Updates expectations + adds a unit test for default enablement (with Windows-aware expected args). |
| java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java | Updates expectations + adds a unit test for default enablement (with Windows-aware expected args). |
Suppressed comments (2)
py/selenium/webdriver/chrome/service.py:64
- This checks
os.environinstead of the service's configuredenvmapping. If a caller passesenv={"CHROME_LOG_FILE": "..."}to the Service, the driver process will seeCHROME_LOG_FILEbut this method will still add--enable-chrome-logs, overriding the caller's intent. Useself.envhere so the args reflect the environment actually used for the child process.
# skip when CHROME_LOG_FILE is set; --enable-chrome-logs would override the user's log file
args = [] if "CHROME_LOG_FILE" in os.environ else ["--enable-chrome-logs"]
return args + [f"--port={self.port}"] + self._service_args
py/selenium/webdriver/edge/service.py:64
- This checks
os.environinstead of the service's configuredenvmapping. If a caller passesenv={"CHROME_LOG_FILE": "..."}to the Service, the driver process will seeCHROME_LOG_FILEbut this method will still add--enable-chrome-logs, overriding the caller's intent. Useself.envhere so the args reflect the environment actually used for the child process.
# yes, it is --enable-chrome-logs, even on msedgedriver; skip when CHROME_LOG_FILE is set
args = [] if "CHROME_LOG_FILE" in os.environ else ["--enable-chrome-logs"]
return args + [f"--port={self.port}"] + self._service_args
c38c1df to
0b486d4
Compare
|
Code review by qodo was updated up to the latest commit 0b486d4 |
…LE is set or on Windows (SeleniumHQ#16201)
0b486d4 to
070ee82
Compare
🔗 Related Issues
Fixes #16201
💥 What does this PR do?
--enable-chrome-logsto chromedriver/msedgedriver on startup by default (matching current .NET and Python code), so the browser's stdio is captured in the driver log instead of leaking to the user's console.CHROME_LOG_FILEenvironment variable when set, so it no longer silently overrides the user's Chrome log file. This also fixes a pre-existing override in .NET and Python.🔧 Implementation Notes
logPathingoog:chromeOptions, butCHROME_LOG_FILE🤖 AI assistance
💡 Additional Considerations
--enable-chrome-logsas an opt-inenableChromeLogging()method rather than forcing it as default; should consider updating it to match the other bindings🔄 Types of changes
CHROME_LOG_FILE.--enable-chrome-logsby default.