Skip to content

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

Merged
titusfortner merged 1 commit into
SeleniumHQ:trunkfrom
titusfortner:enable-chrome-logs
Aug 3, 2026
Merged

[dotnet][java][py][rb] pass --enable-chrome-logs unless CHROME_LOG_FILE is set#17858
titusfortner merged 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 (2) 📎 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 ChromiumService env gate untested 📘 Rule violation ☼ Reliability ⭐ New
Description
The .NET ChromiumDriverService now conditionally adds --enable-chrome-logs based on
CHROME_LOG_FILE, but this behavior change is not covered by a targeted test. Without test
coverage, future refactors can easily reintroduce the override bug this PR is addressing.
Code

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

+            if (Environment.GetEnvironmentVariable("CHROME_LOG_FILE") is null)
+            {
+                argsBuilder.Append(" --enable-chrome-logs");
+            }
Evidence
PR Compliance ID 4 requires adding/updating tests for behavior changes. The changed code adds an
environment-variable conditional that affects runtime arguments, but no corresponding test changes
are present to assert the new behavior.

AGENTS.md: Add or Update Tests for Fixes and Prefer Small Unit Tests Over Browser Tests
dotnet/src/webdriver/Chromium/ChromiumDriverService.cs[177-182]

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 .NET `ChromiumDriverService` now skips appending `--enable-chrome-logs` when `CHROME_LOG_FILE` is set, but there is no test validating the resulting `CommandLineArguments`.

## Issue Context
This PR changes default driver startup arguments to avoid overriding a user-provided Chrome log file; tests should lock in the intended behavior.

## Fix Focus Areas
- dotnet/src/webdriver/Chromium/ChromiumDriverService.cs[177-182]

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


3. Python EdgeService untested env gate 📘 Rule violation ☼ Reliability ⭐ New
Description
Edge and Chrome service argument generation now conditionally omits --enable-chrome-logs when
CHROME_LOG_FILE is set, but there are no corresponding tests asserting this behavior. This risks
regressions where browser logs are unexpectedly redirected or not redirected depending on
environment.
Code

py/selenium/webdriver/edge/service.py[R62-64]

+        # 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
Evidence
PR Compliance ID 4 requires behavior changes to be covered by tests. The new logic gates
--enable-chrome-logs on CHROME_LOG_FILE, but the existing service tests shown only cover
SE_EDGEDRIVER and SE_CHROMEDRIVER environment handling and do not assert any
CHROME_LOG_FILE-dependent behavior, leaving the change unverified.

AGENTS.md: Add or Update Tests for Fixes and Prefer Small Unit Tests Over Browser Tests
py/selenium/webdriver/edge/service.py[61-64]
py/test/selenium/webdriver/edge/edge_service_tests.py[145-166]
py/selenium/webdriver/chrome/service.py[61-64]
py/test/selenium/webdriver/chrome/chrome_service_tests.py[145-166]

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

## Issue description
Python `selenium.webdriver.edge.service.Service.command_line_args()` and `selenium.webdriver.chrome.service.Service.command_line_args()` now skip `--enable-chrome-logs` when `CHROME_LOG_FILE` is present, but current tests do not verify this behavior.

## Issue Context
This PR changes default logging behavior and relies on an environment variable guard; without tests, this can regress silently depending on CI/user environments.

## Fix Focus Areas
- py/selenium/webdriver/edge/service.py[62-64]
- py/test/selenium/webdriver/edge/edge_service_tests.py[145-166]
- py/selenium/webdriver/chrome/service.py[62-64]
- py/test/selenium/webdriver/chrome/chrome_service_tests.py[145-166]

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


4. .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


View more (1)
5. 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

6. 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.

Previous review results

Review updated until commit 070ee82

Results up to commit 8ce198b ⚖️ Balanced


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


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


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
Comment thread py/selenium/webdriver/edge/service.py
Comment thread dotnet/src/webdriver/Chromium/ChromiumDriverService.cs
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit c38c1df

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-logs by default for Java and Ruby driver services (Chrome/Edge), aligning behavior across bindings.
  • Stop overriding user intent when CHROME_LOG_FILE is 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.environ instead of the service's configured env mapping. If a caller passes env={"CHROME_LOG_FILE": "..."} to the Service, the driver process will see CHROME_LOG_FILE but this method will still add --enable-chrome-logs, overriding the caller's intent. Use self.env here 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.environ instead of the service's configured env mapping. If a caller passes env={"CHROME_LOG_FILE": "..."} to the Service, the driver process will see CHROME_LOG_FILE but this method will still add --enable-chrome-logs, overriding the caller's intent. Use self.env here 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

Comment thread rb/lib/selenium/webdriver/edge/service.rb
Comment thread py/selenium/webdriver/edge/service.py Outdated
Comment thread py/selenium/webdriver/chrome/service.py Outdated
Comment thread java/src/org/openqa/selenium/chrome/ChromeDriverService.java
Comment thread java/src/org/openqa/selenium/edge/EdgeDriverService.java
Comment thread rb/lib/selenium/webdriver/chrome/service.rb
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0b486d4

@titusfortner
titusfortner merged commit 10eeb94 into SeleniumHQ:trunk Aug 3, 2026
7 checks passed
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

3 participants