Skip to content

[py] run affected python tests per-browser on PR, full suite on schedule#17643

Open
titusfortner wants to merge 1 commit into
trunkfrom
py-affected-split
Open

[py] run affected python tests per-browser on PR, full suite on schedule#17643
titusfortner wants to merge 1 commit into
trunkfrom
py-affected-split

Conversation

@titusfortner
Copy link
Copy Markdown
Member

@titusfortner titusfortner commented Jun 5, 2026

🔗 Related Issues

Part of #17539 to minimize low signal tests and duplication with RBE.

💥 What does this PR do?

Makes the Python GitHub Actions workflow run only the tests a PR or Push actually affects,
instead of the full suite on every PR. Continues to run existing tests on schedule/manual kickoff

  • ci.yml now passes the affected //py target list into ci-python.yml to run only those targets for PRs and Pulls.
  • Scheduled and Manual runs will continue to run the same browser targets
  • Rely on RBE to provide unit test coverage, and move the matrix to run on schedule/manual
  • Lint is kept per-PR, but build is moved to scheduled/manual
  • Trusts the Affected Targets code and doesn't override when [py] is specified in PR message or commit

🔧 Implementation Notes

  • using an intersection query command to limit changes
  • rerun-failures.sh is shared infra (all bindings). The change is additive and
    backward-compatible — it only affects run commands with a trailing $(...), which no other
    binding uses today.
  • rerun-failures.sh updated to support analyzing query results

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the ci.yml / ci-python.yml workflow changes and the
      rerun-failures.sh edit, iterated against repeated review.
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Any additional work to improve how bazel targets are siloed will improve this more
  • Plan is to switch the per-PR filtering from bazel query intersect to --test_tag_filters after implementing the necessary tags

🔄 Types of changes

  • CI / tooling change (non-breaking; no production code or public API affected)

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Jun 5, 2026
@titusfortner titusfortner marked this pull request as ready for review June 5, 2026 19:40
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 5, 2026

Review Summary by Qodo

(Agentic_describe updated until commit 79bfb73)

Run affected Python tests per-browser on PR, full suite on schedule

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Run only affected Python tests per-browser on PR, full suite on schedule
• Pass target list from main CI workflow to Python workflow via inputs
• Update rerun-failures script to handle bazel query results
• Remove build job dependency for test jobs, keep only on schedule/manual
• Use bazel query intersection to filter tests by affected targets
Diagram
flowchart LR
  CI["ci.yml<br/>Read Targets"] -->|"py_targets"| PY["ci-python.yml<br/>Receive targets input"]
  PY -->|"Schedule/Manual"| BUILD["Build Job"]
  PY -->|"PR: affected only"| REMOTE["Remote Tests<br/>bazel query intersect"]
  PY -->|"PR: affected only"| BROWSER["Browser Tests<br/>bazel query intersect"]
  RERUN["rerun-failures.sh<br/>Parse query results"] -.->|"Enhanced"| REMOTE
  RERUN -.->|"Enhanced"| BROWSER

Loading

Grey Divider

File Changes

1. scripts/github-actions/rerun-failures.sh ✨ Enhancement +2/-1

Support bazel query results in rerun script

• Enhanced sed command to strip both literal targets (//...) and trailing bazel query expressions
 ($(...))
• Added comment explaining the target stripping logic
• Maintains backward compatibility with existing bindings

scripts/github-actions/rerun-failures.sh


2. .github/workflows/ci-python.yml ✨ Enhancement +19/-7

Filter Python tests by affected targets per-browser

• Added targets input parameter to workflow_call for receiving affected targets from main CI
• Restricted build job to schedule/manual events only via conditional
• Restricted unit-tests job to schedule/manual events only
• Removed build job dependency from remote-tests and browser-tests jobs
• Updated remote-tests to use bazel query intersection with affected targets
• Updated browser-tests to use bazel query intersection with affected targets
• Changed run commands to use bazel-test-if-targets.sh helper script

.github/workflows/ci-python.yml


3. .github/workflows/ci.yml ✨ Enhancement +5/-3

Pass affected Python targets to Python workflow

• Renamed output variable from py to py_targets for clarity
• Changed Python binding processing from check_binding to process_binding to handle multiple
 targets
• Pass py_targets output to Python workflow via new targets input parameter
• Updated Python job conditional to use renamed py_targets output

.github/workflows/ci.yml


Grey Divider

Qodo Logo

@titusfortner titusfortner marked this pull request as draft June 5, 2026 19:40
@titusfortner titusfortner marked this pull request as ready for review June 5, 2026 19:41
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 5, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2)

Grey Divider


Action required

1. Build job skips scheduled runs 📘 Rule violation ☼ Reliability ⭐ New
Description
In .github/workflows/ci-python.yml, both the build and unit-tests jobs are gated on
github.event_name == 'schedule', but the workflow is only invoked via
workflow_call/workflow_dispatch from .github/workflows/ci.yml, so on scheduled CI the called
workflow’s event name is workflow_call and these jobs will be skipped. This can silently eliminate
intended scheduled build and unit-test coverage and reduce CI safety.
Code

.github/workflows/ci-python.yml[17]

+    if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
Evidence
The citations indicate that ci-python.yml is a reusable workflow triggered by workflow_call (and
workflow_dispatch), yet its build and unit-tests jobs use an if condition checking for
github.event_name == 'schedule' (optionally allowing workflow_dispatch). Since scheduled
execution happens in ci.yml (which contains the schedule trigger) and it invokes ci-python.yml
via uses: ./.github/workflows/ci-python.yml, the event context inside the called workflow is
workflow_call, making the schedule-based condition false and causing both jobs to not run during
scheduled CI.

.github/workflows/ci-python.yml[3-18]
.github/workflows/ci.yml[3-12]
.github/workflows/ci.yml[121-128]
.github/workflows/ci-python.yml[3-9]
.github/workflows/ci-python.yml[48-52]
Best Practice: Learned patterns

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

## Issue description
In the reusable workflow `.github/workflows/ci-python.yml`, the `build` and `unit-tests` jobs are gated by `if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'`. When this workflow is executed via `workflow_call` from `.github/workflows/ci.yml` (including scheduled runs), `github.event_name` in the called workflow evaluates to `workflow_call`, so these jobs are skipped unintentionally.

## Issue Context
`.github/workflows/ci.yml` is the workflow that triggers on `schedule` and calls `ci-python.yml` via `uses: ./.github/workflows/ci-python.yml`. The called workflow currently has no `schedule` trigger and thus cannot ever have `github.event_name == 'schedule'`; the condition in the called workflow therefore does not reflect the caller’s trigger and prevents the scheduled build and unit-test coverage from running.

## Fix Focus Areas
- .github/workflows/ci-python.yml[15-18]
- .github/workflows/ci-python.yml[48-61]
- .github/workflows/ci.yml[3-12]
- .github/workflows/ci.yml[121-128]

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


2. [py] override no longer works 🐞 Bug ☼ Reliability
Description
ci.yml no longer honors [py] in PR titles/commit messages to force the Python workflow; it now
only runs Python when py_targets is non-empty. PRs that only touch .github/ or scripts/ can
produce no affected Bazel targets, causing the Python workflow (and thus these CI changes) to be
skipped entirely.
Code

.github/workflows/ci.yml[102]

+          process_binding "//py" "py"
Evidence
check_binding() explicitly checks COMMIT_MESSAGES/PR_TITLE for [tag], but Python now uses
process_binding() which only filters the affected target list and ignores tags; the Python
workflow is then skipped unless py_targets is set. Separately, bazel:affected_targets writes an
empty bazel-targets.txt when no targets are detected, and its directory-based fallback only maps
known top-level binding directories, so changes under .github/ or scripts/ can yield no Python
targets.

.github/workflows/ci.yml[82-103]
.github/workflows/ci.yml[121-127]
rake_tasks/bazel.rake[41-62]
rake_tasks/bazel.rake[234-244]

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 CI is now gated exclusively on `py_targets` being non-empty, but `py_targets` is only populated from affected Bazel targets and does not consider `[py]` tags in PR titles/commit messages. This breaks the long-standing “force-run” behavior and can skip Python CI for workflow/tooling-only PRs.

### Issue Context
`check_binding()` supports tag-based overrides via `COMMIT_MESSAGES`/`PR_TITLE`, but Python no longer uses it. `bazel:affected_targets` can legitimately output an empty target set for changes outside recognized binding directories (e.g., `.github/`, `scripts/`).

### Fix Focus Areas
- .github/workflows/ci.yml[82-128]

### Suggested fix
Implement a tag override for Python similar to `check_binding` behavior by setting `py_targets` when `[py]` is present, e.g.:
- After computing `lang_targets` for `//py`, if `[py]` appears in `COMMIT_MESSAGES` or `PR_TITLE`, write `py_targets=//py/...` (or another appropriate “run all py” target set) to `$GITHUB_OUTPUT`.

This keeps the new affected-target behavior while preserving an explicit escape hatch for CI/workflow-only changes.

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


3. Unvalidated bazel query substitution 📘 Rule violation ☼ Reliability
Description
The workflow suppresses bazel query errors and passes its raw output directly into bazel test
without checking the query exit status or whether any targets were returned. This can mask query
failures, drop key diagnostics, and cause incorrect/unsafe CI behavior such as failing with “no
targets” or running an unintended test set.
Code

.github/workflows/ci-python.yml[R70-75]

+      run: >
+        ./scripts/github-actions/bazel-test-if-targets.sh
+        --keep_going
+        --flaky_test_attempts 3
+        --local_test_jobs 1
+        $(bazel query "tests(//py:test-remote) intersect set(${{ inputs.targets || '//py/...' }})" --output=label 2>/dev/null)
Evidence
PR Compliance ID 10 requires CI automation to validate external command outputs and fail safely, but
the newly added ci-python.yml command substitutions run bazel query with stderr discarded
(2>/dev/null) and immediately feed the result into the test invocation without validating success
or non-emptiness. Because the workflow builds its target list via an intersect query that can
legitimately return an empty set (the affected-targets generator emits concrete _test labels, so
intersecting with a specific suite’s tests(...) may produce no labels), the substitution can
expand to an empty string; the wrapper then proceeds to run bazel test "$@" with only flags and no
target patterns, which Bazel treats as an error instead of a clean skip. Additionally, suppressing
query stderr defeats the reusable bazel.yml workflow’s design of capturing combined stdout/stderr
into build/bazel-console.log, removing the primary diagnostics channel when the query fails and
leading to confusing downstream failures.

.github/workflows/ci-python.yml[70-75]
.github/workflows/ci-python.yml[93-99]
.github/workflows/ci-python.yml[63-76]
.github/workflows/ci-python.yml[77-99]
scripts/github-actions/bazel-test-if-targets.sh[8-20]
rake_tasks/bazel.rake[55-62]
rake_tasks/bazel.rake[75-83]
.github/workflows/bazel.yml[211-223]
Best Practice: Learned patterns

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 CI workflow constructs Bazel test target lists using `$(bazel query ... intersect ...)` while discarding query stderr (`2>/dev/null`) and without checking the query exit status or whether the query returned any targets before invoking `bazel test`. This can hide query failures and cause CI to fail confusingly (e.g., `bazel test` invoked with only flags and no target patterns) or behave unsafely by running an unintended set of tests.

## Issue Context
- The `remote-tests` and `browser-tests` jobs in `ci-python.yml` use command substitution to pass the query output into the Bazel test wrapper.
- The query’s intersection can legitimately be empty because the affected-targets generator emits concrete `_test` labels (not `test_suite`), so intersecting with a particular suite’s `tests(...)` set may yield no labels; this should be handled as a clean “nothing to run” case.
- `bazel-test-if-targets.sh` only special-cases exit code 4 and does not detect “no targets passed”, so it may still run `bazel test` with only `--...` flags.
- The reusable `bazel.yml` workflow captures combined stdout/stderr of the overall `inputs.run` command into `build/bazel-console.log`, but `2>/dev/null` prevents `bazel query` stderr from being captured, removing key diagnostics when queries fail.

## Fix Focus Areas
- .github/workflows/ci-python.yml[63-99]
- .github/workflows/ci-python.yml[70-75]
- .github/workflows/ci-python.yml[93-99]
- scripts/github-actions/bazel-test-if-targets.sh[1-20]
- .github/workflows/bazel.yml[211-223]

ⓘ 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 eb7cf50

Results up to commit 79bfb73


🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0)


Action required
1. Unvalidated bazel query substitution 📘 Rule violation ☼ Reliability
Description
The workflow suppresses bazel query errors and passes its raw output directly into bazel test
without checking the query exit status or whether any targets were returned. This can mask query
failures, drop key diagnostics, and cause incorrect/unsafe CI behavior such as failing with “no
targets” or running an unintended test set.
Code

.github/workflows/ci-python.yml[R70-75]

+      run: >
+        ./scripts/github-actions/bazel-test-if-targets.sh
+        --keep_going
+        --flaky_test_attempts 3
+        --local_test_jobs 1
+        $(bazel query "tests(//py:test-remote) intersect set(${{ inputs.targets || '//py/...' }})" --output=label 2>/dev/null)
Evidence
PR Compliance ID 10 requires CI automation to validate external command outputs and fail safely, but
the newly added ci-python.yml command substitutions run bazel query with stderr discarded
(2>/dev/null) and immediately feed the result into the test invocation without validating success
or non-emptiness. Because the workflow builds its target list via an intersect query that can
legitimately return an empty set (the affected-targets generator emits concrete _test labels, so
intersecting with a specific suite’s tests(...) may produce no labels), the substitution can
expand to an empty string; the wrapper then proceeds to run bazel test "$@" with only flags and no
target patterns, which Bazel treats as an error instead of a clean skip. Additionally, suppressing
query stderr defeats the reusable bazel.yml workflow’s design of capturing combined stdout/stderr
into build/bazel-console.log, removing the primary diagnostics channel when the query fails and
leading to confusing downstream failures.

.github/workflows/ci-python.yml[70-75]
.github/workflows/ci-python.yml[93-99]
.github/workflows/ci-python.yml[63-76]
.github/workflows/ci-python.yml[77-99]
scripts/github-actions/bazel-test-if-targets.sh[8-20]
rake_tasks/bazel.rake[55-62]
rake_tasks/bazel.rake[75-83]
.github/workflows/bazel.yml[211-223]
Best Practice: Learned patterns

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 CI workflow constructs Bazel test target lists using `$(bazel query ... intersect ...)` while discarding query stderr (`2>/dev/null`) and without checking the query exit status or whether the query returned any targets before invoking `bazel test`. This can hide query failures and cause CI to fail confusingly (e.g., `bazel test` invoked with only flags and no target patterns) or behave unsafely by running an unintended set of tests.

## Issue Context
- The `remote-tests` and `browser-tests` jobs in `ci-python.yml` use command substitution to pass the query output into the Bazel test wrapper.
- The query’s intersection can legitimately be empty because the affected-targets generator emits concrete `_test` labels (not `test_suite`), so intersecting with a particular suite’s `tests(...)` set may yield no labels; this should be handled as a clean “nothing to run” case.
- `bazel-test-if-targets.sh` only special-cases exit code 4 and does not detect “no targets passed”, so it may still run `bazel test` with only `--...` flags.
- The reusable `bazel.yml` workflow captures combined stdout/stderr of the overall `inputs.run` command into `build/bazel-console.log`, but `2>/dev/null` prevents `bazel query` stderr from being captured, removing key diagnostics when queries fail.

## Fix Focus Areas
- .github/workflows/ci-python.yml[63-99]
- .github/workflows/ci-python.yml[70-75]
- .github/workflows/ci-python.yml[93-99]
- scripts/github-actions/bazel-test-if-targets.sh[1-20]
- .github/workflows/bazel.yml[211-223]

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


Results up to commit decdc1c


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0)


Action required
1. [py] override no longer works 🐞 Bug ☼ Reliability
Description
ci.yml no longer honors [py] in PR titles/commit messages to force the Python workflow; it now
only runs Python when py_targets is non-empty. PRs that only touch .github/ or scripts/ can
produce no affected Bazel targets, causing the Python workflow (and thus these CI changes) to be
skipped entirely.
Code

.github/workflows/ci.yml[102]

+          process_binding "//py" "py"
Evidence
check_binding() explicitly checks COMMIT_MESSAGES/PR_TITLE for [tag], but Python now uses
process_binding() which only filters the affected target list and ignores tags; the Python
workflow is then skipped unless py_targets is set. Separately, bazel:affected_targets writes an
empty bazel-targets.txt when no targets are detected, and its directory-based fallback only maps
known top-level binding directories, so changes under .github/ or scripts/ can yield no Python
targets.

.github/workflows/ci.yml[82-103]
.github/workflows/ci.yml[121-127]
rake_tasks/bazel.rake[41-62]
rake_tasks/bazel.rake[234-244]

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 CI is now gated exclusively on `py_targets` being non-empty, but `py_targets` is only populated from affected Bazel targets and does not consider `[py]` tags in PR titles/commit messages. This breaks the long-standing “force-run” behavior and can skip Python CI for workflow/tooling-only PRs.

### Issue Context
`check_binding()` supports tag-based overrides via `COMMIT_MESSAGES`/`PR_TITLE`, but Python no longer uses it. `bazel:affected_targets` can legitimately output an empty target set for changes outside recognized binding directories (e.g., `.github/`, `scripts/`).

### Fix Focus Areas
- .github/workflows/ci.yml[82-128]

### Suggested fix
Implement a tag override for Python similar to `check_binding` behavior by setting `py_targets` when `[py]` is present, e.g.:
- After computing `lang_targets` for `//py`, if `[py]` appears in `COMMIT_MESSAGES` or `PR_TITLE`, write `py_targets=//py/...` (or another appropriate “run all py” target set) to `$GITHUB_OUTPUT`.

This keeps the new affected-target behavior while preserving an explicit escape hatch for CI/workflow-only changes.

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


Qodo Logo

Comment thread .github/workflows/ci-python.yml
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 5, 2026

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

Comment thread .github/workflows/ci.yml
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Jun 6, 2026

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

jobs:
build:
name: Build
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
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.

Action required

1. Build job skips scheduled runs 📘 Rule violation ☼ Reliability

In .github/workflows/ci-python.yml, both the build and unit-tests jobs are gated on
github.event_name == 'schedule', but the workflow is only invoked via
workflow_call/workflow_dispatch from .github/workflows/ci.yml, so on scheduled CI the called
workflow’s event name is workflow_call and these jobs will be skipped. This can silently eliminate
intended scheduled build and unit-test coverage and reduce CI safety.
Agent Prompt
## Issue description
In the reusable workflow `.github/workflows/ci-python.yml`, the `build` and `unit-tests` jobs are gated by `if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'`. When this workflow is executed via `workflow_call` from `.github/workflows/ci.yml` (including scheduled runs), `github.event_name` in the called workflow evaluates to `workflow_call`, so these jobs are skipped unintentionally.

## Issue Context
`.github/workflows/ci.yml` is the workflow that triggers on `schedule` and calls `ci-python.yml` via `uses: ./.github/workflows/ci-python.yml`. The called workflow currently has no `schedule` trigger and thus cannot ever have `github.event_name == 'schedule'`; the condition in the called workflow therefore does not reflect the caller’s trigger and prevents the scheduled build and unit-test coverage from running.

## Fix Focus Areas
- .github/workflows/ci-python.yml[15-18]
- .github/workflows/ci-python.yml[48-61]
- .github/workflows/ci.yml[3-12]
- .github/workflows/ci.yml[121-128]

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants