Skip to content

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented Sep 30, 2025

User description

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR Type

Bug fix


Description

  • Add JSON dependency constraint for Java platform

  • Fix unit tests failing in CI pipeline

  • Restrict JSON gem to version <= 2.13.2 on JRuby


Diagram Walkthrough

flowchart LR
  A["Ruby Gemspec"] --> B["Add JSON Dependency"] --> C["Java Platform Constraint"] --> D["Fixed Tests"]
Loading

File Walkthrough

Relevant files
Bug fix
selenium-webdriver.gemspec
Add JSON dependency constraint for Java platform                 

rb/selenium-webdriver.gemspec

  • Add JSON gem dependency with version constraint <= 2.13.2
  • Constraint applies specifically to Java platform (JRuby)
  • Positioned after base64 dependency in gemspec
+1/-0     

@selenium-ci selenium-ci added the C-rb Ruby Bindings label Sep 30, 2025
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

5678 - Not compliant

Non-compliant requirements:

  • Resolve ConnectFailure error with multiple ChromeDriver instances.
  • Ensure subsequent instantiations do not error.
  • Provide/verify reproduction and confirmation of fix.

Requires further human verification:

  • Environment-specific validation on Ubuntu 16.04.4 with specified Chrome/ChromeDriver versions.
  • Reproduction across multiple runs and CI vs local parity checks.

1234 - Not compliant

Non-compliant requirements:

  • Restore click() triggering JavaScript in href for Firefox.

Requires further human verification:

  • Browser-level behavior verification on Firefox versions and Selenium bindings.
⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Dependency Constraint

Verify that constraining the 'json' gem to <= 2.13.2 only on JRuby is sufficient to fix CI failures without impacting MRI users; consider whether additional lower-bound or explicit JRuby version notes are needed.

s.add_dependency 'json', ['<= 2.13.2'], platform: :java
s.add_dependency 'logger', ['~> 1.4']
Platform Scoping

Confirm that 'platform: :java' correctly targets JRuby in gemspec and does not inadvertently skip JRuby on non-standard platforms or affect dependency resolution on other Rubies.

s.add_dependency 'json', ['<= 2.13.2'], platform: :java
s.add_dependency 'logger', ['~> 1.4']

Copy link
Contributor

qodo-merge-pro bot commented Sep 30, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use a correct and secure version range

Correct the non-existent version number for the json gem dependency and use a
secure version range instead of only an upper bound.

rb/selenium-webdriver.gemspec [53]

-s.add_dependency 'json', ['<= 2.13.2'], platform: :java
+s.add_dependency 'json', ['>= 2.0.0', '< 2.4.0'], platform: :java
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that the specified json gem version 2.13.2 does not exist, which would cause dependency resolution failures, and proposes a more robust version range.

High
  • Update

@diemol
Copy link
Member

diemol commented Oct 2, 2025

This is already part of #16332

Thanks @aguspe!

@diemol diemol closed this Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants