[build] Ensure all valid ruby tests are running on RBE #15718
+188
−111
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
selenium-ci
added
C-rb
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
CI Feedback 🧐(Feedback updated until commit 67d372e)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
titusfortner
commented
May 7, 2025
.bazelrc.remote
Outdated
| @@ -11,7 +11,7 @@ build:rbe --define=EXECUTOR=remote | |||
| build:rbe --experimental_inmemory_dotd_files | |||
| build:rbe --experimental_inmemory_jdeps_files | |||
| build:rbe --remote_timeout=3600 | |||
| build:rbe --spawn_strategy=remote,local | |||
| build:rbe --spawn_strategy=remote | |||
There was a problem hiding this comment.
turns out this doesn't fix the problem I thought it would, so I'll revert it.
For some reason, when I'm running --config=rbe anything with tag exclusive-with-local still runs in series instead of parallel. I'm not sure why. @shs96c do you know why this may be?
There was a problem hiding this comment.
(it makes a huge difference time-wise when I remove the exclusive-with-local tags and running on rbe.
There was a problem hiding this comment.
well known issue bazelbuild/bazel#17834
There was a problem hiding this comment.
Ah, so this is probably why we were previously skipping them on rbe to avoid the performance hit
… parallel" This reverts commit ca2b267.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
We've seen a few issues in other bindings for problems hidden by tests in the skipped-test file
💥 What does this PR do?
bidi-remoteruby test targetsbidi-supportedanddev-tools-supportedfor specific browsers.🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement, Bug fix
Description
Refactored Ruby test build logic for better variant handling
bidi-only,remote-only, andno-gridtagsbidi_supported,devtools_supported)Fixed WebSocket connection error handling in Ruby bindings
Thread.stopwithThread.exitfor cleaner thread terminationCleaned up and reorganized Bazel Ruby test targets
.skipped-testsBUILD.bazelfiles for browser-specific test execution and taggingMinor test and helper improvements
Changes walkthrough 📝
10 files
Refactor test target generation and browser capability taggingEnhance wait helper to accept extra optionsAdd/modify test targets for bidi and network; update tagsAdd bidi-only tag to all BiDi test targetsSeparate service test with no-grid/skip-rbe tagsSeparate service test with no-grid/skip-rbe tagsSeparate service test with no-grid/skip-rbe tagsAdd new service test target for IE with no-grid tagAdd grid-only tag to remote test targetsSeparate service test with no-grid tag for Safari5 files
Improve WebSocket thread error handling and cleanupFix test exclusion key for FirefoxUpdate test exclusivity to only Chrome/EdgeImprove waiting logic and test exclusion for downloadsFix file upload parameter handling in Rack server2 files
Remove local spawn strategy from RBE configRemove Ruby integration test targets from skip list1 files