[py] update how python tests are filtered and run on github

[titusfortner]

User description

🔗 Related Issues

Python implementation of #16809

We are mostly relying on RBE to tell us if there is a problem, this is supposed to be extra information that strikes a good balance between information and execution time.

💥 What does this PR do?

• First, I made it so that remote isn't tied to one browser. Bazel supports chrome and firefox, but this is quickly adjustable

• ci.yml kicks off on every commit. It runs the new and improved check-bazel-targets.sh

• Instead of just checking to see if there are any py targets and running the whole ci-python workflow, the unique set of applicable test targets are passed from the script to ci-python.yml and browser tests are only run on those

• Additionally, docs/steep/unit tests are only run if the PR includes "[py]" or is executed manually (passed into ci-python.yml from ci.yml with a run-full-suite argument.

• Even if tagged [py], if only a subset of python tests are matched, only those will be run

• Removed build instead of gating all tests on it. Most of the build is executed by the applicable tests as necessary. If there is something weird that breaks building, it'll get detected in nightly run

• As far as browser tests changed, it removes the edge tests for now for consistency with other bindings

💡 Additional Considerations

We may want to filter this further, and I'd like to add testing on beta versions of chrome/firefox
Is there a reason why the remote tests are running on Windows and not in RBE? Can we remove skip-rbe and try to run more things in RBE?

---

PR Type

Enhancement, Tests

---

Description

• Decouple remote testing from specific browser driver

• Filter Python tests by affected files in CI workflow

• Conditionally run docs/typing/unit tests on [py] tag

• Generate separate remote test targets for Chrome and Firefox

• Remove build job dependency from test execution

---

Diagram Walkthrough

flowchart LR
  A["CI Workflow"] -->|"check-bazel-targets.sh"| B["Affected Files"]
  B -->|"bazel query"| C["Test Targets"]
  C -->|"targets output"| D["ci-python.yml"]
  D -->|"run-full-suite"| E["Docs/Typing/Unit Tests"]
  D -->|"filter-targets"| F["Browser Tests"]
  F -->|"--remote flag"| G["Remote Driver"]
  G -->|"Chrome/Firefox"| H["Test Execution"]

Loading

File Walkthrough

Relevant files

Enhancement

conftest.py Decouple remote testing from driver class                                py/conftest.py Removed remote from static drivers list and added --remote command-line option Decoupled remote testing from driver class, now uses is_remote property Updated _initialize_driver() to use webdriver.Remote() when --remote flag is set Simplified fixture logic in firefox_options and chromium_options to check --remote flag instead of driver class Removed remote-specific option handling and SupportedBidiDrivers.remote entry +33/-31  check-bazel-targets.sh Improve target filtering with universe limits                        scripts/github-actions/check-bazel-targets.sh Added strict error handling with set -euo pipefail Limited universe to binding roots only using BINDINGS_UNIVERSE variable Fixed bazel query to properly map changed files to targets Excluded manual test tags from results Added deduplication of returned targets +23/-14  ci-python.yml Refactor CI workflow with conditional test execution          .github/workflows/ci-python.yml Added workflow inputs for targets and run-full-suite parameters Removed build job dependency from all test jobs Made docs, typing, and unit-tests conditional on run-full-suite input Added new filter-targets job to filter targets for Python Replaced test-remote with test-chrome-remote and test-firefox-remote targets Changed browser matrix from [chrome, firefox, chrome-bidi, edge] to [chrome, firefox, chrome-bidi, chrome-remote] Updated test execution to use tag filters instead of hardcoded test targets +45/-23  ci.yml Add target filtering to Python workflow trigger                    .github/workflows/ci.yml Pass targets and run-full-suite outputs from check job to Python workflow Set run-full-suite to true only for scheduled runs, manual dispatch, or [py] tag in PR title Removed commit message check for [py] tag +9/-1      BUILD.bazel Generate per-browser remote test targets                                  py/BUILD.bazel Replaced single test-remote target with generated targets for Chrome and Firefox Added --remote flag to test arguments instead of --driver=remote Added browser-specific test sources and arguments to remote test targets Added tag filters chrome-remote and firefox-remote for test selection Removed Edge from remote tests for consistency +37/-27

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #16809 🟢 Ensure test results are reported/organized by binding (maintainer ownership). 🔴 Add upstream browser regression detection (beta browser testing). Standardize or add missing target categories per binding (e.g., remote execution, beta versions, etc.). Per-PR coverage plan includes Windows/macOS/Linux GitHub Actions variations, including Selenium Manager and alternate interpreter unit tests where applicable. Track or respect skip mechanisms like skip-rbe and .skipped-tests as part of the plan. ⚪ Keep per-PR CI signal fast and scoped to the change by filtering jobs/tests based on affected Bazel targets. Improve GitHub Actions granularity so changes trigger only the relevant binding/category workflows rather than full suites unnecessarily. Scheduled (GHA) runs should execute broader coverage compared to per-PR filtered runs.
🟡	🎫 #1234
🟡	🎫 #5678
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Incorrect bazel query: The script attempts to map changed files to Bazel targets but runs bazel query with the literal string file (and suppresses errors), so it can silently produce incorrect or empty target sets instead of reliably handling failures and edge cases. Referred Code # Map changed files to target labels bazel_targets=() for file in $affected_files; do if query_output=$(bazel query --keep_going --noshow_progress "file" 2>/dev/null); then bazel_targets+=(${query_output}) fi done Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Remote config validation: When --remote is set the code instantiates webdriver.Remote(**kwargs) without showing how the remote endpoint/capabilities are validated or constrained, which may be safe if configured elsewhere but cannot be confirmed from this diff alone. Referred Code @property def is_remote(self): return self._request.config.getoption("remote") def _initialize_driver(self): kwargs = {} if self.options is not None: kwargs["options"] = self.options if self.is_remote: # Use Remote driver with the specified browser's options return webdriver.Remote(**kwargs) if self.driver_path is not None: kwargs["service"] = self.service return getattr(webdriver, self.driver_class)(**kwargs) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 6f7fd0d

Category	Suggestion	Impact
Possible issue	✅ Provide remote endpoint for driver Suggestion Impact: The commit updated remote driver creation to provide a command_executor endpoint (derived from the started grid server's status_url) and wired the started server fixture into the driver so the correct remote URL is used when is_remote is enabled. code diff: if self.options is not None: kwargs["options"] = self.options if self.is_remote: - # Use Remote driver with the specified browser's options + kwargs["command_executor"] = self._server.status_url[: -len("/status")] return webdriver.Remote(**kwargs) if self.driver_path is not None: kwargs["service"] = self.service @@ -318,12 +323,14 @@ @pytest.fixture -def driver(request): +def driver(request, server): global selenium_driver driver_class = getattr(request, "param", "Chrome").lower() if selenium_driver is None: selenium_driver = Driver(driver_class, request) + if server: + selenium_driver._server = server Pass the remote server URL to webdriver.Remote via the command_executor argument to ensure tests connect to the correct endpoint. py/conftest.py [306-308] if self.is_remote: # Use Remote driver with the specified browser's options - return webdriver.Remote(**kwargs) + remote_url = os.environ.get("SELENIUM_REMOTE_URL", "http://localhost:4444") + return webdriver.Remote(command_executor=remote_url, **kwargs) [Suggestion processed] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that webdriver.Remote() is called without a command_executor, which would cause remote tests to fail by trying to connect to a hardcoded default address instead of the dynamically started grid server.	High
	Force correct browser selection Add the --driver=%s argument to the remote test suites in py/BUILD.bazel to ensure each suite runs against the correct browser. py/BUILD.bazel [528-531] args = [ "--instafail", "--remote", + "--driver=%s" % browser, ] + BROWSERS[browser]["args"], Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly points out that the new remote test suites are missing the --driver argument, which would cause all remote tests to run against the default browser (Chrome) instead of the intended one (e.g., Firefox), leading to incorrect test execution.	High
Learned best practice	Guard PR-only workflow fields Guard access to github.event.pull_request.title so the workflow doesn't rely on a PR-only field for non-PR events; use a null-safe fallback or an explicit PR presence check. .github/workflows/ci.yml [81-93] run-full-suite: >- ${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call' || - contains(github.event.pull_request.title, '[py]') + contains(github.event.pull_request.title || '', '[py]') }} if: > github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call' || contains(needs.check.outputs.targets, '//py') || - contains(github.event.pull_request.title, '[py]') + contains(github.event.pull_request.title || '', '[py]') Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at GitHub Actions integration boundaries (e.g., PR-only fields like github.event.pull_request.title) before using them in expressions.	Low
	Centralize repeated remote-test gating Extract the repeated “remote directory requires --remote” check into a small helper used by driver, firefox_options, and chromium_options to keep the behavior consistent and easier to update. py/conftest.py [332-334] +def _skip_if_remote_tests_without_flag(request): + if request.node.path.parts[-2] == "remote" and not request.config.getoption("remote"): + pytest.skip("Remote tests require the --remote flag") + # skip tests in the 'remote' directory if not running with --remote flag -if request.node.path.parts[-2] == "remote" and not selenium_driver.is_remote: - pytest.skip("Remote tests require the --remote flag") +_skip_if_remote_tests_without_flag(request) Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Reduce duplication by centralizing repeated logic (e.g., remote-test gating checks) into a shared helper.	Low
Update

---

Previous suggestions

Suggestions up to commit 84bc272

Category	Suggestion	Impact
Possible issue	Fix incorrect and inefficient bazel query Fix the broken bazel query by using the correct variable for the filename and improve efficiency by running a single query for all affected files instead of looping. scripts/github-actions/check-bazel-targets.sh [17-21] -for file in $affected_files; do - if query_output=$(bazel query --keep_going --noshow_progress "file" 2>/dev/null); then +if [[ -n "$affected_files" ]]; then + # The query below finds all targets that are "path-dependent" on any of the affected files. + # This is a more robust way to find which targets need to be tested. + query_output=$(bazel query --keep_going --noshow_progress "somepath(//..., set(${affected_files// / }))" 2>/dev/null) + if [[ -n "$query_output" ]]; then bazel_targets+=(${query_output}) fi -done +fi Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical bug where the bazel query uses a literal string "file" instead of the loop variable, and proposes a more correct and efficient solution that is crucial for the CI script's functionality.	High
General	Improve shell script robustness for parsing To prevent potential word-splitting issues, read the targets input into a bash array instead of iterating over an unquoted variable. .github/workflows/ci-python.yml [86-98] run: | - targets="${{ inputs.targets }}" + read -r -a targets <<< "${{ inputs.targets }}" filtered=() - for t in $targets; do + for t in "${targets[@]}"; do [[ "$t" == //py* ]] && filtered+=("$t") done if [ ${#filtered[@]} -eq 0 ]; then echo "targets=//py/..." >> "$GITHUB_OUTPUT" else echo "targets=${filtered[*]}" >> "$GITHUB_OUTPUT" fi Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out a potential word-splitting issue with an unquoted variable, but the risk is low as bazel targets are unlikely to contain spaces, making this a minor improvement for robustness.	Low

[Copilot]

Pull request overview

This PR refactors how Python tests are filtered and executed in GitHub Actions CI to improve efficiency and granularity of test runs.

Key Changes:

• Transforms "remote" from a driver type to a command-line flag (--remote), allowing any browser to run tests against a Selenium Grid
• Enhances check-bazel-targets.sh to intelligently filter test targets based on changed files, enabling targeted test execution
• Modifies CI workflows to conditionally run full test suites (docs, typing, unit tests) only when explicitly requested or when [py] tag is present, while always running affected browser tests

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
scripts/github-actions/check-bazel-targets.sh	Improved Bazel target detection with better error handling and filtering logic for test targets across bindings
py/conftest.py	Removed "remote" as a driver type and added --remote flag; updated fixtures to support running any browser remotely
py/BUILD.bazel	Refactored remote test targets to generate separate test-<browser>-remote targets for chrome and firefox instead of a single generic remote target
.github/workflows/ci.yml	Added targets and run-full-suite parameters to the Python workflow call for conditional test execution
.github/workflows/ci-python.yml	Added input handling, target filtering job, and updated browser test matrix to use tag-based filtering with targeted test execution

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cgoldberg]

I think the Remote tests on RBE are failing because it can't find the JRE to launch the grid server.

The server fixture in conftest.py calls Server.start() from py/selenium/webdriver/remote/server.py.

That just looks for java on the system PATH using shutil.which("java") ... and that fails.

[titusfortner]

that makes sense why it was skip-rbe, then, I was just checking. :)

[titusfortner]

@cgoldberg I think this is a "real" failure and that the current test isn't testing what you intended?
https://github.com/SeleniumHQ/selenium/actions/runs/20672120779/job/59354800126?pr=16814#step:20:4950

[cgoldberg]

I think this is a "real" failure and that the current test isn't testing what you intended?

I'm not sure why it's failing here, and we haven't seen this before.

But I think the test is legit... it's just too strict, and some other socket error may occur so ReadTimeoutError never gets raised.

I think we can just assert than any HTTP error is raised by urllib3 (this should include socket connection errors).

I would change the import from :

from urllib3.exceptions import ReadTimeoutError

to

from urllib3.exceptions import HTTPError

then inside the test_remote_webdriver_with_http_timeout test, change:

with pytest.raises(ReadTimeoutError):

to

with pytest.raises(HTTPError):

Want me to fix it, or do you want to do it in this PR?

[cgoldberg]

Actually, I just looked at it again.. I think that specific test is probably fine. It should fail, and so should every other test in that file because it couldn't reach the Grid server?

https://github.com/SeleniumHQ/selenium/actions/runs/20672120779/job/59354800126?pr=16814#step:20:4774

or maybe this is a different issue just with that proxy test? Something doesn't look right.

[titusfortner]

No, it can reach grid; it is the only test failing (everything else is properly using the grid with this code).

Maybe I don't understand the purpose of the test as written in trunk. It is using the http server for our test pages and passing its address to the remote driver. I don't think that's correct, so I don't think it should be passing in trunk. At least when I rewire it to use a grid server, it isn't doing what I think it should be doing. What is it supposed to be doing?

[titusfortner]

(the rbe failures are different, I'm investigating separately)

[cgoldberg]

oh yeah.. I see that now.. it should be using the grid server address/port in ClientConfig, not the test web server. The test below it in that file has the same issue.

the test_remote_webdriver_with_http_timeout is supposed to be testing that "you can connect to the remote, but a long operation will timeout if the HTTP timeout is set too short". I guess it passes in trunk because the test is incorrect, but it happens to raise the same exception (ReadTimeoutError) that it is looking for.

We should change those tests to use the grid server address/port, and raise the correct exception if an HTTP timeout occurs.

[titusfortner]

This PR adds that logic for the server. The problem is that I think the http config is supposed to tell the driver to use that address, but it does not, it still needs remote_connection. At least, that's my expectation for what http config is supposed to do.