Skip to content

[build] Fix GeckoDriver rejecting system-access via capabilities - #17811

Merged
titusfortner merged 3 commits into
SeleniumHQ:trunkfrom
titusfortner:firefox-allow-system-access
Jul 22, 2026
Merged

[build] Fix GeckoDriver rejecting system-access via capabilities#17811
titusfortner merged 3 commits into
SeleniumHQ:trunkfrom
titusfortner:firefox-allow-system-access

Conversation

@titusfortner

Copy link
Copy Markdown
Member

🔗 Related Issues

Fixes test failures surfaced by #17804 (GeckoDriver 0.37.0 → 0.37.1 pin bump).

💥 What does this PR do?

GeckoDriver 0.37.1 dropped support for the -remote-allow-system-access Firefox argument via moz:firefoxOptions; it must now be passed to GeckoDriver itself as --allow-system-access, which broke the chrome-context tests.

  • Moves the flag from browser options to the driver service in every binding.
  • Adds a way to set it where the args were hardcoded: GeckoDriverService.Builder#withAllowSystemAccess (Java) and FirefoxDriverService.AllowSystemAccess (.NET).
  • Adds a .NET context-switching test (the API shipped untested).

🔧 Implementation Notes

System access can't be granted to a Grid session per-session, so context tests are now local-only:

  • Set --allow-system-access on an explicit local service.
  • rb/py skip on --remote; Java drops the RemoteFirefoxDriverTest context cases (local coverage stays in FirefoxDriverTest); .NET uses a standalone local fixture.
  • Python firefox_service_tests also skip on --remote — local-only, no Grid flavor needed.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: investigation of the CI failure, the fix across java/rb/js/py/dotnet, and this PR description
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Chrome-context switching has no Grid coverage in any binding — inherent to the per-session limitation, not this change.
  • Pre-existing gaps for follow-up: Python generates -remote targets for local-only Firefox tests, and most .NET FirefoxDriverService args are untested.

🔄 Types of changes

  • Bug fix (backwards compatible)

@selenium-ci selenium-ci added C-py Python Bindings C-rb Ruby Bindings C-dotnet .NET Bindings C-java Java Bindings C-nodejs JavaScript Bindings labels Jul 21, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix GeckoDriver system-access flag handling across bindings

🐞 Bug fix 🧪 Tests ✨ Enhancement 🕐 40+ Minutes

Grey Divider

AI Description

• Pass system-access via GeckoDriver service args, not Firefox capabilities
• Add explicit APIs to enable system access in Java and .NET services
• Make context-switching coverage local-only; skip/remove remote Grid variants
Diagram

graph TD
  A["Context tests"] --> B["Binding driver ctor"] --> C["DriverService args"] --> D{{"geckodriver"}} --> E{{"Firefox"}}
  A --> F["Remote/Grid runs"]
  F --> G["Skip / remove context tests"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Always enable --allow-system-access by default
  • ➕ Avoids future test failures when callers rely on chrome context APIs
  • ➖ Security/least-privilege regression for all Firefox users
  • ➖ May be unacceptable for locked-down environments or policy-sensitive runs
2. Auto-enable based on requested capabilities/commands
  • ➕ Keeps default secure while reducing setup friction for callers
  • ➖ Not reliably possible because GeckoDriver requires the flag at service start (pre-session)
  • ➖ Couples driver/service configuration to higher-level API usage heuristics
3. Enable via environment/system property only
  • ➕ No new public API surface
  • ➖ Harder to discover and use per-test/per-run
  • ➖ Less consistent across bindings and build tooling

Recommendation: The PR’s explicit, opt-in service-level switch is the best fit: it matches GeckoDriver’s new requirement (service CLI flag), preserves least-privilege defaults, and provides a clear supported API in each binding. Keeping context-switching tests local-only is appropriate given the per-session limitation on Grid.

Files changed (10) +103 / -54

Enhancement (3) +30 / -2
FirefoxDriverService.csAdd AllowSystemAccess to GeckoDriver service args +12/-0

Add AllowSystemAccess to GeckoDriver service args

• Introduces an AllowSystemAccess boolean on FirefoxDriverService and appends --allow-system-access to the GeckoDriver command line when enabled. Documentation clarifies this cannot be requested via capabilities.

dotnet/src/webdriver/Firefox/FirefoxDriverService.cs

GeckoDriverService.javaExpose withAllowSystemAccess() and emit --allow-system-access +16/-0

Expose withAllowSystemAccess() and emit --allow-system-access

• Adds an allowSystemAccess builder option (withAllowSystemAccess) and includes --allow-system-access in GeckoDriverService arguments when enabled. Javadoc documents that this must be set on the service, not capabilities.

java/src/org/openqa/selenium/firefox/GeckoDriverService.java

test_environment.rbAllow passing an explicit service into driver creation +2/-2

Allow passing an explicit service into driver creation

• Extends the test environment helper to accept a service: parameter and forward it into driver instantiation, enabling specs to supply a configured Firefox service.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb

Bug fix (5) +15 / -52
FirefoxDriverTest.javaRun context test with GeckoDriverService allowSystemAccess +3/-2

Run context test with GeckoDriverService allowSystemAccess

• Updates the local context-switching test to start FirefoxDriver using a GeckoDriverService built with withAllowSystemAccess(true) instead of passing the deprecated Firefox option argument.

java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java

RemoteFirefoxDriverTest.javaRemove remote context-switching tests +0/-41

Remove remote context-switching tests

• Deletes RemoteWebDriverBuilder-based context-switching tests that relied on passing -remote-allow-system-access via capabilities, leaving remote coverage focused on supported features (extensions/screenshots).

java/test/org/openqa/selenium/firefox/RemoteFirefoxDriverTest.java

contextSwitching_test.jsConfigure system access via Firefox ServiceBuilder +2/-3

Configure system access via Firefox ServiceBuilder

• Switches context tests from adding -remote-allow-system-access to FirefoxOptions to instead passing --allow-system-access to a firefox.ServiceBuilder and wiring it via setFirefoxService().

javascript/selenium-webdriver/test/firefox/contextSwitching_test.js

firefox_context_tests.pyMake context tests local-only and pass service_args +6/-3

Make context tests local-only and pass service_args

• Skips context tests when running with --remote and starts Firefox with a Service configured with service_args=["--allow-system-access"], removing the old browser-argument approach.

py/test/selenium/webdriver/firefox/firefox_context_tests.py

driver_spec.rbUse Firefox service args for system access; skip on remote +4/-3

Use Firefox service args for system access; skip on remote

• Updates the Firefox context integration spec to create a local Firefox service with --allow-system-access and skips the test when running against a remote driver (Grid) where per-session enablement is not possible.

rb/spec/integration/selenium/webdriver/firefox/driver_spec.rb

Tests (2) +58 / -0
FirefoxCommandContextTests.csAdd .NET context switching test using local service flag +52/-0

Add .NET context switching test using local service flag

• Adds a new test fixture that starts FirefoxDriver with a local FirefoxDriverService configured with AllowSystemAccess=true, then validates getting/setting chrome vs content command contexts.

dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs

firefox_service_tests.pySkip Firefox service tests on Grid/remote runs +6/-0

Skip Firefox service tests on Grid/remote runs

• Adds an autouse fixture to skip the Firefox service test module when running remotely, since driver-service behavior is only exercised with a local driver process.

py/test/selenium/webdriver/firefox/firefox_service_tests.py

@qodo-code-review

qodo-code-review Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Dotnet test bypasses harness ✓ Resolved 🐞 Bug ☼ Reliability
Description
Both the new .NET FirefoxCommandContextTests and the JS contextSwitching_test.js construct their
own FirefoxDriver service/driver (or ServiceBuilder) instead of using the harness
EnvironmentManager/DriverFactory (or JS testing env) that applies pinned geckodriver/firefox
paths, causing them to bypass runfiles-based configuration and potentially fall back to Selenium
Manager discovery. The .NET test also lacks a skip/guard for Browser.Remote, so it may run in
remote/pinned configurations where local geckodriver/firefox resolution and required service flags
are not valid.
Code

dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[R24-36]

+[TestFixture]
+[IgnoreBrowser(Browser.Chrome)]
+[IgnoreBrowser(Browser.Edge)]
+[IgnoreBrowser(Browser.Safari)]
+public class FirefoxCommandContextTests
+{
+    [Test]
+    public void ShouldGetAndSetContext()
+    {
+        FirefoxDriverService service = FirefoxDriverService.CreateDefaultService();
+        service.AllowSystemAccess = true;
+
+        FirefoxDriver driver = new FirefoxDriver(service);
Evidence
The harness EnvironmentManager/DriverFactory is responsible for reading pinned driver/browser
locations (e.g., DRIVER_SERVICE_LOCATION/BROWSER_LOCATION in .NET and
SE_GECKODRIVER/SE_FIREFOX in Bazel/JS) and for selecting between local and remote execution, but
the .NET test bypasses this by calling FirefoxDriverService.CreateDefaultService() and creating a
FirefoxDriver directly, so pinned runfiles paths can be ignored and the test can still attempt to
execute when the suite is configured as remote. In the JS case, the test replaces the
harness-configured Firefox service with new firefox.ServiceBuilder() without an executable path,
which prevents the harness from applying the pinned SE_GECKODRIVER path; when the service has no
executable, Firefox driver creation can fall back to binary discovery (getBinaryPaths(caps) /
Selenium Manager), making the pinned configuration ineffective and leading to non-hermetic failures
in Bazel/pinned CI where binaries are only available via the runfiles/env configuration.

dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[24-37]
dotnet/test/webdriver/Infrastructure/Environment/EnvironmentManager.cs[55-70]
dotnet/test/webdriver/Infrastructure/Environment/DriverFactory.cs[126-151]
javascript/private/browsers.bzl[31-48]
javascript/selenium-webdriver/testing/index.js[319-330]
javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-48]
javascript/selenium-webdriver/firefox.js[561-590]

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

## Issue description
Some Firefox context-switching tests in both .NET and JS override/bypass the test harness’s centralized driver creation and pinned browser/driver configuration: the .NET `FirefoxCommandContextTests` constructs `FirefoxDriverService.CreateDefaultService()` / `FirefoxDriver` directly and does not skip `Browser.Remote`, and the JS `contextSwitching_test.js` builds a `firefox.ServiceBuilder()` without an explicit executable, overriding the harness-provided pinned service. This can ignore pinned geckodriver/firefox runfiles paths and/or fall back to Selenium Manager discovery, making the tests non-hermetic and unreliable in Bazel/pinned CI and remote/Grid configurations.

## Issue Context
- The .NET harness reads `DRIVER_SERVICE_LOCATION` and `BROWSER_LOCATION`, resolves runfiles paths, and applies them via `EnvironmentManager`/`DriverFactory` when creating drivers, including selecting local vs remote execution.
- Bazel pinned Firefox runs set `SE_GECKODRIVER`/`SE_FIREFOX`, and the JS testing environment consumes those to create a `ServiceBuilder` with an explicit executable path (resolving via runfiles/`locate()`); the context-switching test should preserve that executable path while adding `--allow-system-access`.
- The .NET test requires a locally configured service flag (`--allow-system-access`), so it should not run under remote configurations.

## Fix Focus Areas
- dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[24-51]
- dotnet/test/webdriver/Infrastructure/Environment/EnvironmentManager.cs[55-70]
- dotnet/test/webdriver/Infrastructure/Environment/DriverFactory.cs[126-151]
- javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-48]
- javascript/selenium-webdriver/testing/index.js[319-330]
- javascript/private/browsers.bzl[31-48]

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



Remediation recommended

2. Service kwarg breaks remote ✗ Dismissed 🐞 Bug ≡ Correctness ⭐ New
Description
Ruby’s create_driver! now forwards a non-nil service: keyword into driver construction even when
the selected driver is :remote, but Remote::Driver explicitly rejects service and raises
ArgumentError. Any spec run configured for remote that passes service: (now or in the future)
will fail during driver creation.
Code

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[R214-221]

+        def create_driver!(listener: nil, http_client: nil, service: nil, **, &block)
          check_for_previous_error
          http_client ||= Remote::Http::Default.new(read_timeout: 30)
          @safari_pairing_attempts ||= 0

          method = :"#{driver}_driver"
-          opts = {options: build_options(**), listener: listener, http_client: http_client}
+          opts = {options: build_options(**), listener: listener, http_client: http_client, service: service}.compact
          instance = private_methods.include?(method) ? send(method, **opts) : WebDriver::Driver.for(driver, **opts)
Evidence
create_driver! now includes service: service in the forwarded options, and the remote driver
implementation explicitly raises if service is provided, so a remote run that passes service:
will fail at instantiation time.

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[214-221]
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[291-298]
rb/lib/selenium/webdriver/remote/driver.rb[34-54]

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

### Issue description
`create_driver!` adds `service:` to the options hash and forwards it to the selected driver. When `WD_SPEC_DRIVER=remote`, this ultimately reaches `Selenium::WebDriver::Remote::Driver`, which rejects `service` and raises `ArgumentError`.

### Issue Context
This PR added `service:` support so Firefox context tests can pass `--allow-system-access` via a local GeckoDriver service. That’s correct for local drivers, but remote/Grid sessions cannot accept a local service object.

### Fix Focus Areas
- rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb[214-221]

### Suggested fix
Adjust `create_driver!` to avoid passing `service` when `driver == :remote` (or explicitly raise a clearer harness error when a service is provided in remote mode). For example:
- Build `opts` without `service` by default.
- Add `opts[:service] = service` only when `service` is non-nil **and** `driver != :remote`.

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


3. AllowSystemAccess lacks unit test 📘 Rule violation ☼ Reliability
Description
The new AllowSystemAccess behavior is only covered via a full browser/integration test, even
though the change is a simple command-line argument toggle that can be validated with a small unit
test. This increases test runtime and makes failures harder to diagnose when GeckoDriver/Firefox
environment changes.
Code

dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[R243-246]

+            if (this.AllowSystemAccess)
+            {
+                argsBuilder.Append(" --allow-system-access");
+            }
Evidence
PR Compliance ID 4 requires adding/updating tests and preferring small unit tests when feasible. The
PR adds --allow-system-access behind AllowSystemAccess in the service command-line builder, but
the only new coverage is an integration test that launches FirefoxDriver to exercise context
switching.

AGENTS.md: Add or Update Tests for Changes; Prefer Small Unit Tests and Avoid Mocks
dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[243-246]
dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[30-46]

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

## Issue description
`FirefoxDriverService.AllowSystemAccess` adds `--allow-system-access` to the GeckoDriver command line, but there is no small/unit test asserting this behavior; current coverage relies on starting a real `FirefoxDriver`.

## Issue Context
Compliance prefers small unit tests when feasible. This change is a deterministic command-line string mutation and can be verified without launching Firefox/GeckoDriver.

## Fix Focus Areas
- dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[243-246]
- dotnet/test/webdriver/Firefox/FirefoxDriverServiceTests.cs[26-100]
- dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[30-46]

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


4. JS remote context test fails ✓ Resolved 🐞 Bug ☼ Reliability
Description
The JS Firefox context-switching test sets --allow-system-access on a local ServiceBuilder, but
when tests run via SELENIUM_REMOTE_URL/SELENIUM_SERVER_JAR the Builder.build() remote path
ignores firefoxService_, so the remote GeckoDriver never receives the flag and
setContext(CHROME) is expected to fail (unless the remote server is specially preconfigured).
Code

javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[R42-43]

+          let service = new firefox.ServiceBuilder().addArguments('--allow-system-access')
+          driver = await env.builder().setFirefoxService(service).build()
Evidence
The test only configures a local GeckoDriver service argument, but Selenium’s JS Builder.build()
uses that service only for local sessions; when configured for remote execution it constructs a
remote Firefox driver and ignores the local service entirely, so the flag cannot be applied in that
mode.

javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-44]
javascript/selenium-webdriver/index.js[639-685]
javascript/selenium-webdriver/testing/index.js[184-201]

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

## Issue description
`javascript/selenium-webdriver/test/firefox/contextSwitching_test.js` configures `--allow-system-access` on a local `firefox.ServiceBuilder`, but when JS tests are executed against a remote server (via `SELENIUM_REMOTE_URL` or `SELENIUM_SERVER_JAR`), `selenium-webdriver` creates a remote session and does not use the local `firefoxService_` (so the flag is never applied). This makes the context-switching tests unreliable/failing in remote/Grid modes.

## Issue Context
- The test suite explicitly supports remote execution using `SELENIUM_REMOTE_URL`/`SELENIUM_SERVER_JAR`.
- In `Builder.build()`, when a remote URL is configured, the code creates a remote `firefox.Driver` and never calls `this.firefoxService_.build()`.

## Fix
Skip (or conditionally disable) the context-switching tests when running in remote mode, similar to the rb/py changes in this PR. A simple approach is to add a check in the `beforeEach` (or wrap the `describe` using the test framework’s ignore helper) that calls `this.skip()` when `process.env.SELENIUM_REMOTE_URL` or `process.env.SELENIUM_SERVER_JAR` is set.

## Fix Focus Areas
- javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-45]

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


Grey Divider

Previous review results

Review updated until commit 66b47b2

Results up to commit 3b8252c ⚖️ Balanced


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


Remediation recommended
1. JS remote context test fails ✓ Resolved 🐞 Bug ☼ Reliability
Description
The JS Firefox context-switching test sets --allow-system-access on a local ServiceBuilder, but
when tests run via SELENIUM_REMOTE_URL/SELENIUM_SERVER_JAR the Builder.build() remote path
ignores firefoxService_, so the remote GeckoDriver never receives the flag and
setContext(CHROME) is expected to fail (unless the remote server is specially preconfigured).
Code

javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[R42-43]

+          let service = new firefox.ServiceBuilder().addArguments('--allow-system-access')
+          driver = await env.builder().setFirefoxService(service).build()
Evidence
The test only configures a local GeckoDriver service argument, but Selenium’s JS Builder.build()
uses that service only for local sessions; when configured for remote execution it constructs a
remote Firefox driver and ignores the local service entirely, so the flag cannot be applied in that
mode.

javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-44]
javascript/selenium-webdriver/index.js[639-685]
javascript/selenium-webdriver/testing/index.js[184-201]

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

## Issue description
`javascript/selenium-webdriver/test/firefox/contextSwitching_test.js` configures `--allow-system-access` on a local `firefox.ServiceBuilder`, but when JS tests are executed against a remote server (via `SELENIUM_REMOTE_URL` or `SELENIUM_SERVER_JAR`), `selenium-webdriver` creates a remote session and does not use the local `firefoxService_` (so the flag is never applied). This makes the context-switching tests unreliable/failing in remote/Grid modes.

## Issue Context
- The test suite explicitly supports remote execution using `SELENIUM_REMOTE_URL`/`SELENIUM_SERVER_JAR`.
- In `Builder.build()`, when a remote URL is configured, the code creates a remote `firefox.Driver` and never calls `this.firefoxService_.build()`.

## Fix
Skip (or conditionally disable) the context-switching tests when running in remote mode, similar to the rb/py changes in this PR. A simple approach is to add a check in the `beforeEach` (or wrap the `describe` using the test framework’s ignore helper) that calls `this.skip()` when `process.env.SELENIUM_REMOTE_URL` or `process.env.SELENIUM_SERVER_JAR` is set.

## Fix Focus Areas
- javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-45]

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


Results up to commit 776477e ⚖️ Balanced


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


Action required
1. Dotnet test bypasses harness ✓ Resolved 🐞 Bug ☼ Reliability
Description
Both the new .NET FirefoxCommandContextTests and the JS contextSwitching_test.js construct their
own FirefoxDriver service/driver (or ServiceBuilder) instead of using the harness
EnvironmentManager/DriverFactory (or JS testing env) that applies pinned geckodriver/firefox
paths, causing them to bypass runfiles-based configuration and potentially fall back to Selenium
Manager discovery. The .NET test also lacks a skip/guard for Browser.Remote, so it may run in
remote/pinned configurations where local geckodriver/firefox resolution and required service flags
are not valid.
Code

dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[R24-36]

+[TestFixture]
+[IgnoreBrowser(Browser.Chrome)]
+[IgnoreBrowser(Browser.Edge)]
+[IgnoreBrowser(Browser.Safari)]
+public class FirefoxCommandContextTests
+{
+    [Test]
+    public void ShouldGetAndSetContext()
+    {
+        FirefoxDriverService service = FirefoxDriverService.CreateDefaultService();
+        service.AllowSystemAccess = true;
+
+        FirefoxDriver driver = new FirefoxDriver(service);
Evidence
The harness EnvironmentManager/DriverFactory is responsible for reading pinned driver/browser
locations (e.g., DRIVER_SERVICE_LOCATION/BROWSER_LOCATION in .NET and
SE_GECKODRIVER/SE_FIREFOX in Bazel/JS) and for selecting between local and remote execution, but
the .NET test bypasses this by calling FirefoxDriverService.CreateDefaultService() and creating a
FirefoxDriver directly, so pinned runfiles paths can be ignored and the test can still attempt to
execute when the suite is configured as remote. In the JS case, the test replaces the
harness-configured Firefox service with new firefox.ServiceBuilder() without an executable path,
which prevents the harness from applying the pinned SE_GECKODRIVER path; when the service has no
executable, Firefox driver creation can fall back to binary discovery (getBinaryPaths(caps) /
Selenium Manager), making the pinned configuration ineffective and leading to non-hermetic failures
in Bazel/pinned CI where binaries are only available via the runfiles/env configuration.

dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[24-37]
dotnet/test/webdriver/Infrastructure/Environment/EnvironmentManager.cs[55-70]
dotnet/test/webdriver/Infrastructure/Environment/DriverFactory.cs[126-151]
javascript/private/browsers.bzl[31-48]
javascript/selenium-webdriver/testing/index.js[319-330]
javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-48]
javascript/selenium-webdriver/firefox.js[561-590]

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

## Issue description
Some Firefox context-switching tests in both .NET and JS override/bypass the test harness’s centralized driver creation and pinned browser/driver configuration: the .NET `FirefoxCommandContextTests` constructs `FirefoxDriverService.CreateDefaultService()` / `FirefoxDriver` directly and does not skip `Browser.Remote`, and the JS `contextSwitching_test.js` builds a `firefox.ServiceBuilder()` without an explicit executable, overriding the harness-provided pinned service. This can ignore pinned geckodriver/firefox runfiles paths and/or fall back to Selenium Manager discovery, making the tests non-hermetic and unreliable in Bazel/pinned CI and remote/Grid configurations.

## Issue Context
- The .NET harness reads `DRIVER_SERVICE_LOCATION` and `BROWSER_LOCATION`, resolves runfiles paths, and applies them via `EnvironmentManager`/`DriverFactory` when creating drivers, including selecting local vs remote execution.
- Bazel pinned Firefox runs set `SE_GECKODRIVER`/`SE_FIREFOX`, and the JS testing environment consumes those to create a `ServiceBuilder` with an explicit executable path (resolving via runfiles/`locate()`); the context-switching test should preserve that executable path while adding `--allow-system-access`.
- The .NET test requires a locally configured service flag (`--allow-system-access`), so it should not run under remote configurations.

## Fix Focus Areas
- dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[24-51]
- dotnet/test/webdriver/Infrastructure/Environment/EnvironmentManager.cs[55-70]
- dotnet/test/webdriver/Infrastructure/Environment/DriverFactory.cs[126-151]
- javascript/selenium-webdriver/test/firefox/contextSwitching_test.js[40-48]
- javascript/selenium-webdriver/testing/index.js[319-330]
- javascript/private/browsers.bzl[31-48]

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



Remediation recommended
2. AllowSystemAccess lacks unit test 📘 Rule violation ☼ Reliability
Description
The new AllowSystemAccess behavior is only covered via a full browser/integration test, even
though the change is a simple command-line argument toggle that can be validated with a small unit
test. This increases test runtime and makes failures harder to diagnose when GeckoDriver/Firefox
environment changes.
Code

dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[R243-246]

+            if (this.AllowSystemAccess)
+            {
+                argsBuilder.Append(" --allow-system-access");
+            }
Evidence
PR Compliance ID 4 requires adding/updating tests and preferring small unit tests when feasible. The
PR adds --allow-system-access behind AllowSystemAccess in the service command-line builder, but
the only new coverage is an integration test that launches FirefoxDriver to exercise context
switching.

AGENTS.md: Add or Update Tests for Changes; Prefer Small Unit Tests and Avoid Mocks
dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[243-246]
dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[30-46]

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

## Issue description
`FirefoxDriverService.AllowSystemAccess` adds `--allow-system-access` to the GeckoDriver command line, but there is no small/unit test asserting this behavior; current coverage relies on starting a real `FirefoxDriver`.

## Issue Context
Compliance prefers small unit tests when feasible. This change is a deterministic command-line string mutation and can be verified without launching Firefox/GeckoDriver.

## Fix Focus Areas
- dotnet/src/webdriver/Firefox/FirefoxDriverService.cs[243-246]
- dotnet/test/webdriver/Firefox/FirefoxDriverServiceTests.cs[26-100]
- dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs[30-46]

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


Qodo Logo

Comment thread javascript/selenium-webdriver/test/firefox/contextSwitching_test.js Outdated

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 Firefox “system access” enablement to match GeckoDriver 0.37.1’s behavior change by moving the flag from Firefox capabilities/options to the GeckoDriver service layer across bindings, and adjusts/limits affected tests when running on Grid.

Changes:

  • Ruby/Python/JS tests now request system access via the local GeckoDriver service (--allow-system-access) and skip remote/Grid where this cannot be applied per-session.
  • Java adds GeckoDriverService.Builder#withAllowSystemAccess and updates local context-switching coverage; remote-context cases are removed.
  • .NET adds FirefoxDriverService.AllowSystemAccess, wires it into GeckoDriver args, and adds a regression test for command context switching.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb Allows passing a driver service: into integration test driver creation.
rb/spec/integration/selenium/webdriver/firefox/driver_spec.rb Switches context test to local GeckoDriver --allow-system-access and skips Grid.
py/test/selenium/webdriver/firefox/firefox_service_tests.py Skips Firefox service tests on Grid where local service behavior isn’t exercised.
py/test/selenium/webdriver/firefox/firefox_context_tests.py Uses local GeckoDriver service args for system access and skips Grid runs.
javascript/selenium-webdriver/test/firefox/contextSwitching_test.js Uses a Firefox ServiceBuilder with --allow-system-access for context tests.
java/test/org/openqa/selenium/firefox/RemoteFirefoxDriverTest.java Removes remote-context-switching coverage that can’t be supported per-session.
java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java Uses the new GeckoDriver service flag for local context switching test coverage.
java/src/org/openqa/selenium/firefox/GeckoDriverService.java Adds withAllowSystemAccess and emits --allow-system-access when enabled.
dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs Adds coverage for get/set Firefox command context with system access enabled.
dotnet/src/webdriver/Firefox/FirefoxDriverService.cs Adds AllowSystemAccess and appends --allow-system-access to GeckoDriver args.

Comment thread javascript/selenium-webdriver/test/firefox/contextSwitching_test.js Outdated
Comment thread dotnet/src/webdriver/Firefox/FirefoxDriverService.cs
Comment thread dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 776477e

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment thread dotnet/test/webdriver/Firefox/FirefoxCommandContextTests.cs
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 66b47b2

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-nodejs JavaScript Bindings C-py Python Bindings C-rb Ruby Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants