[rb] fix unit tests by aguspe · Pull Request #16357 · SeleniumHQ/selenium

aguspe · 2025-09-29T16:37:08Z

User description

This pull request makes a small change to the gem specification by adding a new dependency.

Added the prism gem (version ~> 1.0, < 1.5) as a dependency in rb/selenium-webdriver.gemspec to ensure compatibility with JRuby

🔄 Types of changes

Bug fix (backwards compatible)

PR Type

Bug fix

Description

Add prism dependency constraint to Ruby gemspec
Fix version compatibility issues in unit tests
Restrict prism to versions 1.0 to 1.5

Diagram Walkthrough

flowchart LR
  A["Ruby Gemspec"] --> B["Add prism dependency"] --> C["Fix Unit Tests"]

File Walkthrough

Relevant files

Bug fix

selenium-webdriver.gemspec `Add prism dependency constraint` rb/selenium-webdriver.gemspec Add `prism` gem dependency with version constraint `~> 1.0, < 1.5` Fix dependency compatibility issues for Ruby unit tests	+1/-0

qodo-code-review · 2025-09-29T16:37:33Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1234 - Partially compliant Compliant requirements: Non-compliant requirements: Investigate and fix regression where click() no longer triggers JavaScript in link href in Firefox (2.48.x vs 2.47.1). Ensure behavior works on Firefox 42.0 (32-bit) per report. Provide tests or validation demonstrating the fix. Requires further human verification: Manual validation on affected Firefox version to confirm click() triggers JavaScript in href. Cross-version/browser regression testing to ensure no behavior changes elsewhere. 5678 - Partially compliant Compliant requirements: Non-compliant requirements: Diagnose "ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances. Provide a code fix or configuration change to prevent connection failures after the first instance. Add tests or reproducible steps validating multiple instances work reliably. Requires further human verification: Runtime verification by launching multiple ChromeDriver instances across environments.
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Version Constraint The prism dependency is capped to < 1.5 while the description claims 1.0 to 1.5; verify whether 1.5.x should be allowed (<= 1.5) or intentionally excluded (< 1.5) to avoid unintended incompatibilities. s.add_dependency 'prism', ['~> 1.0', '< 1.5'] s.add_dependency 'rexml', ['~> 3.2', '>= 3.2.5'] Dependency Impact Adding prism as a runtime dependency may affect downstream consumers; confirm it is required at runtime vs development/test only, and ensure compatibility with supported Ruby versions. s.add_dependency 'prism', ['~> 1.0', '< 1.5'] s.add_dependency 'rexml', ['~> 3.2', '>= 3.2.5']

qodo-code-review · 2025-09-29T16:38:43Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use development dependency for test-only gems If the `prism` gem is only for testing, change its declaration from a runtime dependency (`add_dependency`) to a development dependency (`add_development_dependency`). rb/selenium-webdriver.gemspec [54] -s.add_dependency 'prism', ['~> 1.0', '< 1.5'] +s.add_development_dependency 'prism', ['~> 1.0', '< 1.5'] Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue where a development dependency is declared as a runtime dependency, which is an important best practice for gem maintainability and user experience.	Medium
More

exterm · 2025-10-03T14:45:43Z

why restrict prism to < 1.5 ? This means we need to downgrade prism in our app to upgrade selenium-webdriver.

G-Rath · 2025-10-05T19:51:17Z

Similarly it looks like v4.36.0 (#16332) has done something similar with the json gem - neither of these should be needed by this gem as it is not responsible for ensuring the versions installed are compatible with specific platforms, that's the role of the downstream consumer and their bundler; instead the gem should just be saying what versions it is compatible with.

I can confirm that I've been using selenium-webdriver without issue on the latest versions of prism and json without issue on MRI Ruby for a while

diemol · 2025-10-06T11:03:30Z

You both have a point. The issue we faced is in our build (we need to use JRuby); the dependencies showed a conflict with Rubocop. We will check this again when releasing 4.37.

* update devtools versions * update selenium manager versions * update maven dependency versions * update authors file * bump versions in preparation for release * WIP - rough auto-update of changelog, please edit * Update sha256 for signed selenium-manager-windows.exe artifact * Updating tests and linting readme. * Updating Gemfile.lock * Rolling back json upgrade * Pinning rust lock * Pinning rust lock * Removing guards and switching to macOS [skip ci] * Removing guards --------- Co-authored-by: Selenium CI Bot <selenium-ci@users.noreply.github.com> Co-authored-by: Diego Molina <diemol@users.noreply.github.com> Co-authored-by: Boni Garcia <boni.garcia@uc3m.es> Co-authored-by: Diego Molina <diemol@gmail.com>

This project does not use prism at all. At most it is being used by rubocop during local development. It was added in SeleniumHQ#16357 but it is unclear to me why. I just bumped it for this repo, perhaps that JRuby issue has since been fixed. Either way, it should not be part of the gemspec.

* [rb] Remove `prism` dependency This project does not use prism at all. At most it is being used by rubocop during local development. It was added in #16357 but it is unclear to me why. I just bumped it for this repo, perhaps that JRuby issue has since been fixed. Either way, it should not be part of the gemspec. * Pin prism to 1.4.0 for the project It fails to build with the following error: src/prism.c: In function ‘context_terminator’: src/prism.c:8651:62: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result [-Werror=sign-conversion] 8651 | return token->type < 32 && (context_terminators[context] & (1 << token->type)); | ^ cc1: all warnings being treated as errors --------- Co-authored-by: Diego Molina <diemol@users.noreply.github.com>

Changes: https://github.com/SeleniumHQ/selenium/releases https://github.com/SeleniumHQ/selenium/blob/trunk/rb/CHANGES Reference: SeleniumHQ/selenium#16332 SeleniumHQ/selenium#16357

titusfortner and others added 15 commits March 22, 2025 11:54

[build] allow tests tagged exclusive-if-local to run on rbe

8c317d8

merge trunk

bb29c66

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

5b6a0ce

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

c9cb77c

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

ac9a80e

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

aef922a

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

8dc6cfc

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

16dc0d3

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

85fdc1c

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

72bf4ec

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

81f2331

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

d03e95b

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

0cc0626

Merge branch 'trunk' of github.com:aguspe/selenium into trunk

efa60a2

decrease prism

ec6c82e

selenium-ci added the C-rb Ruby Bindings label Sep 29, 2025

qodo-code-review bot added the Review effort 1/5 label Sep 29, 2025

aguspe changed the title ~~Rb fix unit tests~~ [rb] fix unit tests Sep 29, 2025

diemol merged commit 4c603ec into SeleniumHQ:trunk Sep 29, 2025
22 of 25 checks passed

Earlopain mentioned this pull request Oct 15, 2025

[rb] Remove prism dependency #16437

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[rb] fix unit tests#16357

[rb] fix unit tests#16357
diemol merged 15 commits intoSeleniumHQ:trunkfrom
aguspe:rb_fix_unit_tests

aguspe commented Sep 29, 2025 •

edited

Loading

Uh oh!

qodo-code-review bot commented Sep 29, 2025

Uh oh!

qodo-code-review bot commented Sep 29, 2025

Uh oh!

Uh oh!

exterm commented Oct 3, 2025

Uh oh!

G-Rath commented Oct 5, 2025

Uh oh!

diemol commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

aguspe commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Sep 29, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Sep 29, 2025

PR Code Suggestions ✨

Uh oh!

Uh oh!

exterm commented Oct 3, 2025

Uh oh!

G-Rath commented Oct 5, 2025

Uh oh!

diemol commented Oct 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aguspe commented Sep 29, 2025 •

edited

Loading