Skip to content

[rb] add guard for browser version#15899

Merged
aguspe merged 21 commits intoSeleniumHQ:trunkfrom
aguspe:rb_add_guard_for_browser_version
Jun 19, 2025
Merged

[rb] add guard for browser version#15899
aguspe merged 21 commits intoSeleniumHQ:trunkfrom
aguspe:rb_add_guard_for_browser_version

Conversation

@aguspe
Copy link
Contributor

@aguspe aguspe commented Jun 14, 2025 •
edited by qodo-code-review bot
Loading

User description

💥 What does this PR do?

This PR adds support to exclude tests based on the browser version

🔧 Implementation Notes

I implemented this way to leverage existent browser information on the test environment

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement

Description

• Add browser version guard support for test exclusion
• Integrate version condition into test guards system
• Apply version exclusion to FedCM spec tests

Changes walkthrough 📝

Relevant files
Tests
fedcm_spec.rb
Add version exclusion to FedCM tests                                         

rb/spec/integration/selenium/webdriver/fedcm_spec.rb

• Add version exclusion parameter to FedCM test describe block
•
Exclude specific Chrome version 138.0.7204.23 from tests

 +1/-0     
Enhancement
spec_helper.rb
Register version guard condition                                                 

rb/spec/integration/selenium/webdriver/spec_helper.rb

• Add version condition to guards system
• Register browser_version
from GlobalTestEnv as guard condition

 +1/-0     
test_environment.rb
Add browser version accessor method                                           

rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb

• Add browser_version method to access driver capabilities
• Refactor
current_env to use new browser_version method
• Extract browser
version logic into dedicated method

 +5/-1     
Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @selenium-ci selenium-ci added the C-rb Ruby Bindings label Jun 14, 2025
    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 14, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Potential Error

    The browser_version method calls driver_instance.capabilities.browser_version without checking if driver_instance is initialized. This could cause errors if called before driver creation.

     
    def browser_version
  driver_instance.capabilities.browser_version
end
    Hard-coded Version

    The excluded Chrome version is hard-coded which may require maintenance when new problematic versions are discovered. Consider if this approach scales well for version management.

     
    exclude: {version: '138.0.7204.23'},
exclusive: [{bidi: false, reason: 'Not yet implemented with BiDi'}, {browser: %i[chrome edge]}] do

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 14, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add nil safety checks

    Add nil check for driver_instance to prevent potential NoMethodError when driver
    is not initialized. This ensures the method gracefully handles cases where
    capabilities might not be available.

    rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb [60-62]

     def browser_version
-  driver_instance.capabilities.browser_version
+  driver_instance&.capabilities&.browser_version
 end
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion correctly identifies a potential NoMethodError if driver_instance is nil. While the surrounding code likely ensures driver_instance is initialized, adding the safe navigation operator (&.) is a good defensive practice that improves code robustness.

    		Low
    • Update

    @aguspe aguspe changed the title Rb add guard for browser version [rb] add guard for browser version Jun 15, 2025
    @aguspe aguspe merged commit 71ad272 into SeleniumHQ:trunk Jun 19, 2025
    22 of 24 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    C-rb Ruby Bindings Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    3 participants

    @aguspe @selenium-ci @titusfortner