Skip to content

[rb] add guard for browser version #15899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jun 19, 2025

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented Jun 14, 2025

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
    Copy link
    Contributor

    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

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 14, 2025

    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
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants