Skip to content

[dotnet][java][py][rb] pass --enable-chrome-logs unless CHROME_LOG_FILE is set - #17858

Open
titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:enable-chrome-logs
Open

[dotnet][java][py][rb] pass --enable-chrome-logs unless CHROME_LOG_FILE is set#17858
titusfortner wants to merge 1 commit into
SeleniumHQ:trunkfrom
titusfortner:enable-chrome-logs

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Fixes #16201

💥 What does this PR do?

  • Java and Ruby now pass --enable-chrome-logs to 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.
  • Java, Ruby, .NET, Python respect CHROME_LOG_FILE environment 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

  • Technically this is not backwards compatible for Java users setting logPath in goog:chromeOptions, but
    • This setting is only documented briefly in one place (the Logging page)
    • users are unlikely to be setting it, and there's no easy way for us to check the capabilities from the Service classes.
    • Python & .NET already decided to break potential backwards compatibility so this is just matching
    • Ruby doesn't currently support custom vendor options at all right now, anyway
  • The explicitly documented way to save the browser log is to use CHROME_LOG_FILE
    • Fixes Python & .NET to allow its usage
    • Supports usage in Java & Ruby
    • JavaScript does its own thing (see Additional Considerations)

🤖 AI assistance

  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the binding changes and test updates
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • JavaScript exposes --enable-chrome-logs as an opt-in enableChromeLogging() method rather than forcing it as default; should consider updating it to match the other bindings

🔄 Types of changes

  • Bug fix (backwards compatible): .NET/Python no longer override a user's CHROME_LOG_FILE.
  • New feature: Java/Ruby now pass --enable-chrome-logs by default.

@selenium-ci selenium-ci added C-py Python Bindings C-rb Ruby Bindings C-dotnet .NET Bindings C-java Java Bindings labels Aug 2, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Respect CHROME_LOG_FILE and default --enable-chrome-logs across bindings

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Enable Chrome/Edge browser stdio capture via driver logs by default (Java/Ruby).
• Stop overriding user-defined CHROME_LOG_FILE in .NET/Python/Java/Ruby.
• Update unit tests to assert new default driver argument behavior.
Diagram

graph TD
  A["Binding Service"] --> B{"CHROME_LOG_FILE set?"} -->|"no"| C["Add --enable-chrome-logs"] --> D["Launch chromedriver/msedgedriver"] --> E["Driver log captures browser stdio"]
  B -->|"yes"| F["Skip enable flag"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Expose an explicit API toggle (opt-in/opt-out) per binding
  • ➕ Avoids any backwards-compat surprises from changing defaults
  • ➕ More discoverable than relying on CHROME_LOG_FILE semantics
  • ➖ Requires API surface changes across bindings and documentation updates
  • ➖ Still needs a default choice; inconsistency risk across languages
2. Detect user chromeOptions logPath capability and conditionally omit the flag
  • ➕ Preserves existing users who set Chrome logging via capabilities
  • ➕ Keeps default behavior for everyone else
  • ➖ Service classes often can’t reliably inspect effective capabilities at that layer
  • ➖ Adds coupling between service startup and capability parsing across bindings
3. Keep behavior opt-in like JavaScript’s enableChromeLogging()
  • ➕ Most conservative change; avoids changing driver output by default
  • ➕ Matches an existing pattern in one binding
  • ➖ Leaves console log leakage as the default for Java/Ruby
  • ➖ Maintains cross-binding inconsistency

Recommendation: Proceed with the PR’s approach: default to --enable-chrome-logs for better UX, but gate it on CHROME_LOG_FILE to preserve the explicitly documented user override. It achieves cross-binding consistency with minimal API churn; if further compatibility concerns arise, the next step would be adding an explicit toggle rather than trying to infer intent from capabilities.

Files changed (12) +83 / -28

Enhancement (4) +16 / -2
ChromeDriverService.javaAdd --enable-chrome-logs by default (Chrome) unless CHROME_LOG_FILE +4/-0

Add --enable-chrome-logs by default (Chrome) unless CHROME_LOG_FILE

• Adds --enable-chrome-logs to the default chromedriver argument list. Skips the flag when CHROME_LOG_FILE is set to avoid overriding user logging configuration.

java/src/org/openqa/selenium/chrome/ChromeDriverService.java

EdgeDriverService.javaAdd --enable-chrome-logs by default (Edge) unless CHROME_LOG_FILE +4/-0

Add --enable-chrome-logs by default (Edge) unless CHROME_LOG_FILE

• Adds --enable-chrome-logs to msedgedriver args (flag name is still chrome). Skips the flag when CHROME_LOG_FILE is set to preserve user-defined Chrome logging behavior.

java/src/org/openqa/selenium/edge/EdgeDriverService.java

service.rbDefault --enable-chrome-logs for Chrome service unless CHROME_LOG_FILE +4/-1

Default --enable-chrome-logs for Chrome service unless CHROME_LOG_FILE

• Ensures service args always include --enable-chrome-logs by default, without duplicating it when already provided. Skips adding the flag when CHROME_LOG_FILE is set to preserve user logging.

rb/lib/selenium/webdriver/chrome/service.rb

service.rbDefault --enable-chrome-logs for Edge service unless CHROME_LOG_FILE +4/-1

Default --enable-chrome-logs for Edge service unless CHROME_LOG_FILE

• Adds --enable-chrome-logs to msedgedriver args by default, avoiding duplicates. Skips when CHROME_LOG_FILE is set to avoid overriding user log routing.

rb/lib/selenium/webdriver/edge/service.rb

Bug fix (3) +14 / -5
ChromiumDriverService.csOnly enable chrome logs when CHROME_LOG_FILE is unset +6/-2

Only enable chrome logs when CHROME_LOG_FILE is unset

• Makes --enable-chrome-logs conditional on CHROME_LOG_FILE not being present. Prevents the driver from overriding a user-selected Chrome log file destination.

dotnet/src/webdriver/Chromium/ChromiumDriverService.cs

service.pyRespect CHROME_LOG_FILE when building chromedriver args +4/-1

Respect CHROME_LOG_FILE when building chromedriver args

• Conditionally adds --enable-chrome-logs only when CHROME_LOG_FILE is not present in the environment. Avoids overriding the user’s Chrome log file selection.

py/selenium/webdriver/chrome/service.py

service.pyRespect CHROME_LOG_FILE when building msedgedriver args +4/-2

Respect CHROME_LOG_FILE when building msedgedriver args

• Conditionally adds --enable-chrome-logs (same flag name) only when CHROME_LOG_FILE is not set. Prevents unintended override of user Chrome logging configuration.

py/selenium/webdriver/edge/service.py

Tests (5) +53 / -21
ChromeDriverServiceTest.javaUpdate ChromeDriverService tests for default enable-chrome-logs +15/-5

Update ChromeDriverService tests for default enable-chrome-logs

• Adjusts expected argument ordering to include --enable-chrome-logs. Adds a dedicated test asserting the flag is enabled by default.

java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java

EdgeDriverServiceTest.javaUpdate EdgeDriverService tests for default enable-chrome-logs +15/-5

Update EdgeDriverService tests for default enable-chrome-logs

• Adjusts expected argument ordering to include --enable-chrome-logs. Adds a dedicated test asserting the flag is enabled by default.

java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java

service_spec.rbAssert Chrome service enables chrome logs by default (and no duplicates) +10/-4

Assert Chrome service enables chrome logs by default (and no duplicates)

• Updates defaults to expect --enable-chrome-logs in extra_args. Adds coverage to ensure providing the flag does not result in duplication and adjusts other arg expectations accordingly.

rb/spec/unit/selenium/webdriver/chrome/service_spec.rb

service_spec.rbUpdate common service factory expectations for default enable-chrome-logs +2/-2

Update common service factory expectations for default enable-chrome-logs

• Updates factory tests to expect Chrome/Edge services to include --enable-chrome-logs appended to provided args.

rb/spec/unit/selenium/webdriver/common/service_spec.rb

service_spec.rbAssert Edge service enables chrome logs by default (and no duplicates) +11/-5

Assert Edge service enables chrome logs by default (and no duplicates)

• Updates defaults to expect --enable-chrome-logs in extra_args. Adds duplication protection coverage and updates arg expectations for log path and custom args cases.

rb/spec/unit/selenium/webdriver/edge/service_spec.rb

@qodo-code-review

qodo-code-review Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Ruby service nil dup ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Ruby Chrome::Service and Edge::Service now call args.dup even when args is nil, which raises
TypeError and breaks default Service.new() construction. This is a deterministic runtime failure for
the common no-args path.
Code

rb/lib/selenium/webdriver/chrome/service.rb[R30-33]

+          args = Array(args.dup)
+          # skip when CHROME_LOG_FILE is set; --enable-chrome-logs would override the user's log file
+          args << '--enable-chrome-logs' unless ENV.key?('CHROME_LOG_FILE') || args.include?('--enable-chrome-logs')
+
Evidence
Both services default args to nil but immediately call args.dup, which raises before
Selenium::WebDriver::Service can apply its own args ||= [] defaulting.

rb/lib/selenium/webdriver/chrome/service.rb[29-37]
rb/lib/selenium/webdriver/edge/service.rb[29-37]
rb/lib/selenium/webdriver/common/service.rb[69-86]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`args = Array(args.dup)` crashes when `args` is `nil` (default), raising `TypeError: can't dup NilClass`.

## Issue Context
Both Chrome and Edge Ruby services take `args: nil` by default, and should safely construct an argument array before adding `--enable-chrome-logs`.

## Fix Focus Areas
- rb/lib/selenium/webdriver/chrome/service.rb[29-40]
- rb/lib/selenium/webdriver/edge/service.rb[29-40]

## Suggested fix
- Replace `Array(args.dup)` with a nil-safe copy, e.g. `args = Array(args).dup` (or `args = Array(args || []).dup`).
- Ensure the mutated `args` is what gets passed to the parent initializer (e.g. pass `args:` explicitly to `super` if needed), so the appended `--enable-chrome-logs` actually reaches `Selenium::WebDriver::Service`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. .NET ignores StartInfo env ✗ Dismissed 🐞 Bug ☼ Reliability
Description
Across .NET, Java, and Python driver services, the decision to add the default
--enable-chrome-logs flag is made by checking the parent process environment
(Environment.GetEnvironmentVariable / System.getenv / os.environ) rather than the effective
environment actually used to launch the driver process (set via DriverProcessStarting,
Builder.withEnvironment, or the Service’s env). As a result, CHROME_LOG_FILE provided through
these per-process mechanisms is not honored, so --enable-chrome-logs may still be passed and can
override the caller’s intended log file behavior.
Code

dotnet/src/webdriver/Chromium/ChromiumDriverService.cs[R177-182]

+            // Redirect browser logs to the driver log, unless the user set CHROME_LOG_FILE, which
+            // --enable-chrome-logs would otherwise override.
+            if (Environment.GetEnvironmentVariable("CHROME_LOG_FILE") is null)
+            {
+                argsBuilder.Append(" --enable-chrome-logs");
+            }
Evidence
In .NET, ChromiumDriverService reads CHROME_LOG_FILE from the current process environment when
building arguments, but DriverService computes StartInfo.Arguments and only then raises the
public DriverProcessStarting hook that allows consumers to mutate ProcessStartInfo.Environment,
so the argument decision can diverge from the environment actually given to the driver process. In
Java, ChromeDriverService/EdgeDriverService use System.getenv("CHROME_LOG_FILE") inside
createArgs(), while the runtime environment for the launched process can be provided via
DriverService.Builder.withEnvironment(...) and passed through
createDriverService/DriverService separately, meaning createArgs() may not see
CHROME_LOG_FILE that is present for the subprocess. In Python, the base Service uses self.env
(caller-provided env or inherited) for the subprocess environment, but the CHROME_LOG_FILE guard
in command_line_args uses global os.environ, so it can disagree with the effective environment
mapping passed to the driver.

dotnet/src/webdriver/Chromium/ChromiumDriverService.cs[172-184]
dotnet/src/webdriver/DriverService.cs[228-244]
dotnet/src/webdriver/DriverProcessStartingEventArgs.cs[35-46]
java/src/org/openqa/selenium/chrome/ChromeDriverService.java[295-303]
java/src/org/openqa/selenium/edge/EdgeDriverService.java[296-303]
java/src/org/openqa/selenium/remote/service/DriverService.java[362-431]
java/src/org/openqa/selenium/remote/service/DriverService.java[522-526]
py/selenium/webdriver/chrome/service.py[41-65]
py/selenium/webdriver/edge/service.py[37-65]
py/selenium/webdriver/common/service.py[52-80]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Chrome/Edge driver services decide whether to add the default `--enable-chrome-logs` flag by consulting the parent process environment (`Environment.GetEnvironmentVariable("CHROME_LOG_FILE")` / `System.getenv("CHROME_LOG_FILE")` / `os.environ`) rather than the effective environment that will actually be used for the launched driver process (as configured via .NET `DriverProcessStarting` mutations, Java `DriverService.Builder.withEnvironment(...)`, or Python Service `env`). This causes `CHROME_LOG_FILE` supplied through these APIs to be ignored, so `--enable-chrome-logs` may still be added and can override or interfere with the caller’s intended log file selection.

## Issue Context
- **.NET**: `DriverService.StartAsync` sets `StartInfo.Arguments` and then raises `DriverProcessStarting`, allowing consumers to alter the `ProcessStartInfo` (including environment) after arguments are computed.
- **Java**: `DriverService.Builder` stores an `environment` map and passes it into `createDriverService(..., environment)`, which is then applied to the launched process, but `createArgs()` currently consults `System.getenv` only.
- **Python**: `selenium.webdriver.common.service.Service` stores the effective environment in `self.env = env or os.environ`, but Chrome/Edge service `command_line_args` consults `os.environ` directly.

## Fix Focus Areas
- dotnet/src/webdriver/Chromium/ChromiumDriverService.cs[105-185]
- dotnet/src/webdriver/DriverService.cs[228-244]
- dotnet/src/webdriver/DriverProcessStartingEventArgs.cs[35-46]
- java/src/org/openqa/selenium/chrome/ChromeDriverService.java[295-305]
- java/src/org/openqa/selenium/edge/EdgeDriverService.java[296-305]
- java/src/org/openqa/selenium/remote/service/DriverService.java[362-431]
- java/src/org/openqa/selenium/remote/service/DriverService.java[522-526]
- py/selenium/webdriver/chrome/service.py[41-65]
- py/selenium/webdriver/edge/service.py[37-65]
- py/selenium/webdriver/common/service.py[52-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. spy() used in added test ✗ Dismissed 📘 Rule violation ☼ Reliability
Description
The PR adds new Java unit tests that rely on Mockito spy()/verify() rather than exercising real
implementations. This violates the repository compliance requirement to avoid mocks in tests because
mocked interactions can diverge from real driver-service behavior.
Code

java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[R82-85]

+    ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
+
+    builderMock.usingPort(1).build();
+    verify(builderMock)
Evidence
PR Compliance ID 4 prohibits introducing or expanding mock usage in tests. The newly added
enablesChromeLogsByDefault tests use Mockito (spy(...), verify(...), and matchers like
any()) to assert calls instead of validating concrete behavior.

AGENTS.md: Avoid Mocks in Tests
java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[81-88]
java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java[81-88]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New/modified tests introduce Mockito mocks/spies, which is disallowed by the compliance rule "Avoid Mocks in Tests".

## Issue Context
The added tests (`enablesChromeLogsByDefault`) validate argument construction via `spy()`/`verify()` rather than asserting behavior using real objects or fakes, increasing the chance tests pass while real behavior changes.

## Fix Focus Areas
- java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[80-88]
- java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java[80-88]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

4. Java tests depend on env ✗ Dismissed 🐞 Bug ⚙ Maintainability
Description
Java unit tests now hardcode expectations that --enable-chrome-logs is always present, but
production code explicitly omits it when CHROME_LOG_FILE is set. This makes the tests
environment-dependent and can fail on machines/CI where CHROME_LOG_FILE is inherited by the JVM.
Code

java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[R81-88]

+  void enablesChromeLogsByDefault() {
+    ChromeDriverService.Builder builderMock = spy(ChromeDriverService.Builder.class);
+
+    builderMock.usingPort(1).build();
+    verify(builderMock)
+        .createDriverService(
+            any(), anyInt(), any(), eq(List.of("--port=1", "--enable-chrome-logs")), any());
+  }
Evidence
The tests always expect the flag, while the implementation explicitly skips adding it when
CHROME_LOG_FILE is set in the environment.

java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[59-88]
java/src/org/openqa/selenium/chrome/ChromeDriverService.java[295-303]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tests assume `--enable-chrome-logs` is always added, but `createArgs()` conditionally adds it based on `System.getenv("CHROME_LOG_FILE")`. If the environment variable is set in the test JVM environment, assertions will fail.

## Issue Context
Java tests typically cannot portably mutate `System.getenv()` at runtime; the robust approach is to make the code consult a controllable environment (e.g., the builder environment map) or explicitly test the conditional via APIs you control.

## Fix Focus Areas
- java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java[59-88]
- java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java[59-88]
- java/src/org/openqa/selenium/chrome/ChromeDriverService.java[295-303]
- java/src/org/openqa/selenium/edge/EdgeDriverService.java[296-303]

## Suggested fix
- After fixing the production issue to consider `Builder.withEnvironment(...)`, update tests to:
 - Assert default behavior with no CHROME_LOG_FILE in the builder environment.
 - Add a test that sets `withEnvironment(Map.of("CHROME_LOG_FILE", "..."))` and asserts the flag is omitted.
- This removes dependence on the host/JVM inherited environment.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java
Comment thread rb/lib/selenium/webdriver/chrome/service.rb
Comment thread dotnet/src/webdriver/Chromium/ChromiumDriverService.cs
Comment thread java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings C-java Java Bindings C-py Python Bindings C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: Add --enable-chrome-logs flag to chromedriver startup in all bindings

2 participants