Skip to content

Conversation

@aguspe
Copy link
Contributor

@aguspe aguspe commented Jun 21, 2024

Description

This PR updates the ruby example for the safari technological preview in all the languages

Motivation and Context

It's important that all the examples are up to date and they clearly reflect the usage of Selenium Webdriver

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Enhancement, Documentation


Description

  • Added a new test in Ruby to verify setting Safari Technology Preview.
  • Updated documentation in multiple languages (English, Japanese, Portuguese, Chinese) to link to the new Ruby test for Safari Technology Preview.

Changes walkthrough 📝

Relevant files
Enhancement
safari_spec.rb
Add test for Safari Technology Preview in Ruby                     

examples/ruby/spec/browsers/safari_spec.rb

  • Added a test for setting Safari Technology Preview.
  • Verified the browser name is 'Safari Technology Preview'.
  • +6/-0     
    Documentation
    safari.en.md
    Update Ruby example link for Safari Technology Preview     

    website_and_docs/content/documentation/webdriver/browsers/safari.en.md

  • Updated Ruby example to link to the new Safari Technology Preview
    test.
  • +1/-1     
    safari.ja.md
    Update Ruby example link for Safari Technology Preview     

    website_and_docs/content/documentation/webdriver/browsers/safari.ja.md

  • Updated Ruby example to link to the new Safari Technology Preview
    test.
  • +1/-1     
    safari.pt-br.md
    Update Ruby example link for Safari Technology Preview     

    website_and_docs/content/documentation/webdriver/browsers/safari.pt-br.md

  • Updated Ruby example to link to the new Safari Technology Preview
    test.
  • +1/-1     
    safari.zh-cn.md
    Update Ruby example link for Safari Technology Preview     

    website_and_docs/content/documentation/webdriver/browsers/safari.zh-cn.md

  • Updated Ruby example to link to the new Safari Technology Preview
    test.
  • +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @aguspe aguspe marked this pull request as ready for review June 21, 2024 13:31
    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 21, 2024
    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests Yes
    🔒 Security concerns No
    ⚡ Key issues to review None

    @qodo-merge-pro
    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add an ensure block to quit the driver and prevent resource leaks

    To ensure the local_driver is properly closed after the test, consider adding an ensure
    block to quit the driver. This will help prevent resource leaks.

    examples/ruby/spec/browsers/safari_spec.rb [37-38]

     local_driver = Selenium::WebDriver.for :safari
    -expect(local_driver.capabilities.browser_name).to eq 'Safari Technology Preview'
    +begin
    +  expect(local_driver.capabilities.browser_name).to eq 'Safari Technology Preview'
    +ensure
    +  local_driver.quit
    +end
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding an ensure block to quit the driver is a good practice to prevent resource leaks, especially in test environments where many instances might be created and destroyed.

    7
    Possible issue
    Add a check to ensure the technology_preview! method is available before calling it

    To make the test more robust, add a check to ensure that technology_preview! method is
    available before calling it. This will prevent potential errors if the method is not
    defined.

    examples/ruby/spec/browsers/safari_spec.rb [36]

    -Selenium::WebDriver::Safari.technology_preview!
    +if Selenium::WebDriver::Safari.respond_to?(:technology_preview!)
    +  Selenium::WebDriver::Safari.technology_preview!
    +else
    +  skip 'Safari Technology Preview is not available'
    +end
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Checking if a method exists before calling it can prevent runtime errors, making the test more robust. However, this is a precautionary measure rather than a correction of an existing error.

    6

    Copy link
    Member

    @harsha509 harsha509 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thank you @aguspe !

    @harsha509 harsha509 merged commit 4d766e0 into SeleniumHQ:trunk Jun 21, 2024
    selenium-ci added a commit that referenced this pull request Jun 21, 2024
    … site]
    
    Updates the safari technology example for ruby
    
    Co-authored-by: aguspe <[email protected]> 4d766e0
    @aguspe
    Copy link
    Contributor Author

    aguspe commented Jun 21, 2024

    Thank you @aguspe !

    Thank you as usual @harsha509 !

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 2

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants