[bazel] Use MRI on RBE (rebased #17622) by aguspe · Pull Request #18123 · SeleniumHQ/selenium

aguspe · 2026-10-04T22:25:10Z

🔗 Related Issues

This continues the work in #17622

💥 What does this PR do?

Fixes the RBE failures it was hitting, so we can see if it goes green.

The RBE job was failing on the Ruby Firefox specs and the atoms closure tests, all on relative locators: topLeft/topRight and bottomLeft/bottomRight came back swapped. The relative locator atom sorts matches by pixel distance to the anchor, and the cells in relative_locators.html had no fixed width, so their size came from the font.

After some investigation I found out that the new Ubuntu 22 image ships different fonts than the old one, so Firefox rendered the table wrong.

🔧 Implementation Notes

Gave every td in relative_locators.html and in the atoms relative_locator_test.html a fixed width: 20px. I did it this way to fix the issues with the gird test.

🤖 AI assistance

No substantial AI assistance used
AI assisted (complete below)
- Tool(s): Claude Code
- What was generated: It help me with the tests, the investigation and the review
- I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

Breaking change (fix or feature that would cause existing functionality to change)

qodo-code-review · 2026-10-04T22:25:22Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

p0deje

…y in the Ruby unit test matrix

qodo-code-review · 2026-10-10T19:12:24Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Two wrapper comments narrate basic flow~~ ✗ Dismissed

Description

In cc_wrapper.sh, # output is coming and # parse the option list restate the assignment and
loop immediately below them. When a reader changes response-file parsing, neither comment explains
why the wrapper tracks -o across arguments from @ files.

Code

common/remote-build/cc/cc_wrapper.sh[33]
+# parse the option list

Evidence
Rule 6 rejects new comments that merely restate code. The cited comments label an immediately
following assignment and argument loop without adding a rationale.
AGENTS.md: Write Comments That Explain Why Rather Than Restate Code
common/remote-build/cc/cc_wrapper.sh[25-35]

Agent prompt

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

## Issue description
Two new comments in the compiler wrapper narrate assignments and iteration without explaining the response-file handling.

## Fix Focus Areas
- common/remote-build/cc/cc_wrapper.sh[28-33]

## Recommended Fix
Remove the redundant comments, or replace them with a concise explanation of why output arguments must be found inside response files.

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

2. ~~Module scans write the wrong output~~ ✗ Dismissed

Description

deps_scanner_wrapper.sh directs GCC to write structured dependencies to out.tmp while
redirecting its preprocessed-source output to DEPS_SCANNER_OUTPUT_FILE. Even with a compiler that
accepts the options, a module scan leaves Bazel's declared output containing source text instead of
the dependency information it needs.

Code

common/remote-build/cc/deps_scanner_wrapper.sh[12]
+/usr/bin/gcc -E -x c++ -fmodules-ts -fdeps-file=out.tmp -fdeps-format=p1689r5 "$@" >"$DEPS_SCANNER_OUTPUT_FILE"

Evidence
The toolchain sets the environment variable to its declared output, but the wrapper assigns
out.tmp as GCC's dependency file and redirects stdout to that declared output. GCC documents
-fdeps-file as the destination for structured dependency information.
common/remote-build/cc/deps_scanner_wrapper.sh[10-12]
common/remote-build/cc/cc_toolchain_config.bzl[594-606]
🌐 GCC documents -fdeps-file as the file where structured dependency information is written.

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 scanner writes structured dependency data to a temporary file but puts preprocessed source in Bazel's declared scan output.

## Fix Focus Areas
- common/remote-build/cc/deps_scanner_wrapper.sh[10-12]
- common/remote-build/cc/cc_toolchain_config.bzl[594-606]

## Recommended Fix
Set `-fdeps-file` to `DEPS_SCANNER_OUTPUT_FILE` and send preprocessed output elsewhere; ensure the declared output contains the P1689 dependency data.

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

3. ~~C++ module scans fail with the image compiler~~ ✗ Dismissed

Description

deps_scanner_wrapper.sh passes -fdeps-file and -fdeps-format=p1689r5 to /usr/bin/gcc, but
the Ubuntu 22 toolchain is configured for GCC 11 and GCC introduced those scanning options in
version 14. When a build enables C++ modules and invokes the configured scanner, GCC rejects the
options before producing dependencies.

Code

common/remote-build/cc/deps_scanner_wrapper.sh[12]
+/usr/bin/gcc -E -x c++ -fmodules-ts -fdeps-file=out.tmp -fdeps-format=p1689r5 "$@" >"$DEPS_SCANNER_OUTPUT_FILE"

Evidence
The added wrapper invokes /usr/bin/gcc with the P1689 options; the generated toolchain uses GCC 11
include paths and selects that wrapper for module scanning. GCC's release notes identify these
options as new in GCC 14.
common/remote-build/cc/deps_scanner_wrapper.sh[10-12]
common/remote-build/cc/BUILD[129-135]
common/remote-build/cc/BUILD[180-183]
🌐 GCC 14 release notes say C++ module scanning and the -fdeps-format and -fdeps-file options became available in GCC 14.

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 module scanner passes GCC 14 dependency-scanning options to the GCC 11 compiler configured in the remote image.

## Fix Focus Areas
- common/remote-build/cc/deps_scanner_wrapper.sh[10-12]
- common/remote-build/cc/BUILD[169-183]

## Recommended Fix
Use a compiler that supports P1689 dependency scanning for this action, and keep the scanner command and generated toolchain paths consistent with that compiler.

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

4. ~~JRuby-specific code paths lose integration coverage~~ ✗ Dismissed

Description

Before this PR, ci-rbe.yml ran the whole integration suite under JRuby. The PR switches RBE to MRI
and adds JRuby back only to the unit-test matrix in ci-ruby.yml. JRuby-only branches that the
integration specs exercise, such as SocketPoller in socket_poller.rb (the jruby/jruby#5709
workaround) and the server selection in spec_support/rack_server.rb, no longer run in any CI job, so
a JRuby regression in browser-driven behaviour can now ship without being caught.

Code

.github/workflows/ci-ruby.yml[R40-41]

+  # Minimum MRI is run by os-tests and RBE. This matrix covers the most
+  # recent MRI release, jruby and truffleruby.

Evidence
The removed ruby-version: jruby-10.1.0.0 was the only place JRuby ran integration specs. The new
matrix entry only runs unit tests. Both cited files branch on Platform.jruby? in code that only
integration tests reach.
rb/lib/selenium/webdriver/common/socket_poller.rb[66-72]
rb/spec/integration/selenium/webdriver/spec_support/rack_server.rb[35-40]
.github/workflows/ci-ruby.yml[40-56]

Agent prompt

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

## Issue description
Now that RBE runs MRI, no CI job runs the Ruby integration specs under JRuby. JRuby-specific branches go untested.

## Fix Focus Areas
- .github/workflows/ci-ruby.yml[40-56]

## Recommended Fix
Add a JRuby integration-test job, on a schedule or nightly (e.g. a headless Chrome/Firefox subset with ruby-version jruby-10.1.0.0). Otherwise, state in the workflow comment that JRuby integration coverage was dropped on purpose.

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

Context sources

✅ Web pages:

🌐 How to use modules in c++ 20 using g++?

🌐 GCC 11 documentation — DevDocs

🌐 C++ Modules (Using the GNU Compiler Collection (GCC))

+17 more

Review mode: Auto: 🧠 Deep: Broad toolchain, CI, image, and workflow changes create many independent failure paths.

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

p0deje and others added 5 commits October 5, 2026 00:02

[bazel] Update RBE image to Ubuntu 22

66270be

[bazel] Rebuild cc_toolchain with Bazel 9

4197446

[bazel] Use MRI on RBE

24ebc03

[bazel] Switch to selenium image

b210a62

trying fix

0d353c4

selenium-ci added B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations labels Oct 4, 2026

p0deje approved these changes Oct 5, 2026

View reviewed changes

p0deje mentioned this pull request Oct 5, 2026

[bazel] Use MRI on RBE #17622

Closed

titusfortner added 2 commits October 10, 2026 13:32

Merge origin/trunk into mri-rbe-trying-fix

b3c7da7

[build] drop the remaining JRuby pins from release jobs and keep JRub…

ae40773

…y in the Ruby unit test matrix

qodo-code-review Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread common/remote-build/cc/cc_wrapper.sh

Comment thread common/remote-build/cc/deps_scanner_wrapper.sh

Comment thread common/remote-build/cc/deps_scanner_wrapper.sh

Comment thread .github/workflows/ci-ruby.yml

titusfortner mentioned this pull request Oct 10, 2026

[rb] support serving test pages over HTTPS for certificate and cookie tests #18152

Merged

2 tasks done

titusfortner merged commit f43e7b6 into trunk Oct 12, 2026
55 checks passed

titusfortner deleted the mri-rbe-trying-fix branch October 12, 2026 14:40

Navigation Menu

Repository navigation

[bazel] Use MRI on RBE (rebased #17622) - #18123

[bazel] Use MRI on RBE (rebased #17622)#18123
titusfortner merged 7 commits into
trunkfrom
mri-rbe-trying-fix

aguspe commented Oct 4, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Oct 4, 2026

Uh oh!

p0deje left a comment

Uh oh!

qodo-code-review Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

aguspe commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review Bot commented Oct 4, 2026

Qodo reviews are paused for this user.

Uh oh!

p0deje left a comment

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aguspe commented Oct 4, 2026 •

edited

Loading

qodo-code-review Bot commented Oct 10, 2026 •

edited

Loading