Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 13, 2026

User description

Similar to the recent PRs for the bindings - #16879

💥 What does this PR do?

  • if the input to rerun-failures.sh is the ci-build script reference, then use the standard ci-build settings as the command

I'm honestly not certain we want/need this one, and if it complicates things with RBE.

It works: https://github.com/SeleniumHQ/selenium/actions/runs/20975769113/job/60290021932?pr=16905#step:23:29


PR Type

Enhancement


Description

  • Enable debug output for RBE test reruns via CI workflow

  • Add conditional logic to detect ci-build script invocation

  • Use standard RBE CI configuration for failed test reruns

  • Pass debug flag through rerun-failures script


Diagram Walkthrough

flowchart LR
  A["CI RBE Workflow"] -->|"rerun-with-debug: true"| B["rerun-failures.sh"]
  B -->|"Detect ci-build.sh"| C["Use RBE CI Config"]
  C -->|"Add SE_DEBUG=true"| D["Rerun Failed Tests"]
Loading

File Walkthrough

Relevant files
Enhancement
rerun-failures.sh
Add RBE CI config detection for test reruns                           

scripts/github-actions/rerun-failures.sh

  • Add conditional check to detect if RUN_CMD contains ci-build.sh
    reference
  • Use standard RBE CI bazel configuration when ci-build.sh is detected
  • Fallback to original sed-based command extraction for other scripts
  • Preserve debug environment variable and flaky test retry logic
+5/-1     
Configuration changes
ci-rbe.yml
Enable debug flag for RBE test reruns                                       

.github/workflows/ci-rbe.yml

  • Add new rerun-with-debug: true parameter to bazel workflow call
  • Enable debug output for RBE test reruns in CI pipeline
+1/-0     

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jan 13, 2026
@titusfortner
Copy link
Member Author

Well, it works as expected.

@titusfortner titusfortner marked this pull request as ready for review January 14, 2026 16:18
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Sensitive log exposure

Description: Enabling --test_env=SE_DEBUG=true for reruns may cause tests or tooling to emit additional
diagnostic data into CI logs, which can inadvertently include sensitive information (e.g.,
credentials/URLs/tokens printed by debug logging) and should be verified to not expose
secrets in GitHub Actions output.
rerun-failures.sh [27-30]

Referred Code
echo "Rerunning tests: $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets"
set +e
{
  $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
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: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unvalidated command source: The script builds and executes base_cmd from RUN_CMD without validating presence/format,
so unexpected or empty RUN_CMD values could cause confusing failures or unintended
execution behavior.

Referred Code
if [[ "$RUN_CMD" == *"/ci-build.sh"* ]]; then
  base_cmd="bazel test --config=rbe-ci --build_tests_only --keep_going"
else
  base_cmd=$(echo "$RUN_CMD" | sed 's| //[^ ]*||g')
fi
targets=$(tr '\n' ' ' < build/failures/_run1.txt)
echo "Rerunning tests: $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets"
set +e
{
  $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets

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:
Verbose command echo: The echoed rerun command includes full targets and flags, which is likely fine in CI but
should be confirmed not to expose sensitive/internal-only target paths or parameters when
RUN_CMD content varies.

Referred Code
targets=$(tr '\n' ' ' < build/failures/_run1.txt)
echo "Rerunning tests: $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets"
set +e

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:
Command injection risk: Because base_cmd may be derived from RUN_CMD and is executed via $base_cmd ... without
strict allowlisting, the safety depends on RUN_CMD being trusted and controlled by the
workflow.

Referred Code
if [[ "$RUN_CMD" == *"/ci-build.sh"* ]]; then
  base_cmd="bazel test --config=rbe-ci --build_tests_only --keep_going"
else
  base_cmd=$(echo "$RUN_CMD" | sed 's| //[^ ]*||g')
fi
targets=$(tr '\n' ' ' < build/failures/_run1.txt)
echo "Rerunning tests: $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets"
set +e
{
  $base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets

Learn more about managing compliance generic rules or creating your own custom rules

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

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Prevent splitting target names

Use a bash array to store test targets to prevent word-splitting issues when
they are passed as arguments to the bazel command.

scripts/github-actions/rerun-failures.sh [30]

-$base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 $targets
+read -r -a target_array <<< "$targets"
+$base_cmd --test_env=SE_DEBUG=true --flaky_test_attempts=1 "${target_array[@]}"

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential word-splitting issue with the $targets variable and proposes using a bash array for a more robust way to pass test targets as arguments.

Medium
  • More

@titusfortner titusfortner merged commit 7078836 into trunk Jan 14, 2026
20 checks passed
@titusfortner titusfortner deleted the rerun_rbe branch January 14, 2026 18:13
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 C-py Python Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants