Skip to content

Start adding special features examples #1711

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 13 commits into from
May 11, 2024

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented May 7, 2024

User description

Description

Right now none of the example features under Chrome have examples for users to follow, this PR adds the casting example

Motivation and Context

The goal of this PR is to provide users with an overview of what special features they can use with Chrome

Screenshot 2024-05-08 at 16 43 42

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 Ruby test case for Chrome casting functionality, demonstrating device discovery and tab mirroring.
  • Updated English, Japanese, Portuguese, and Chinese Chrome documentation to reference the new Ruby casting example.

Changes walkthrough 📝

Relevant files
Enhancement
chrome_spec.rb
Add Ruby example for casting in Chrome                                     

examples/ruby/spec/browsers/chrome_spec.rb

  • Added a new test case under 'Special Features' for casting
    functionality in Chrome.
  • The test initializes a Chrome driver, retrieves cast sinks, and
    performs tab mirroring if devices are available.
  • Ensures no exceptions are raised during the start and stop of casting.

  • +13/-0   
    Documentation
    chrome.en.md
    Update English Chrome documentation with new Ruby example

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

  • Updated the Ruby code block reference to point to the new casting
    example.
  • +1/-1     
    chrome.ja.md
    Update Japanese Chrome documentation with new Ruby example

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

  • Updated the Ruby code block reference to point to the new casting
    example.
  • +1/-1     
    chrome.pt-br.md
    Update Portuguese Chrome documentation with new Ruby example

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

  • Updated the Ruby code block reference to point to the new casting
    example.
  • +1/-1     
    chrome.zh-cn.md
    Update Chinese Chrome documentation with new Ruby example

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

  • Updated the Ruby code block reference to point to the new casting
    example.
  • +1/-1     

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

    Copy link

    netlify bot commented May 7, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 6b08dee

    @aguspe aguspe marked this pull request as ready for review May 8, 2024 14:45
    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels May 8, 2024
    Copy link
    Contributor

    qodo-merge-pro bot commented May 8, 2024

    PR Description updated to latest commit (bbdc626)

    Copy link
    Contributor

    qodo-merge-pro bot commented May 8, 2024

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the PR primarily involves adding a new example for a specific feature in Ruby and updating documentation across multiple languages to reference this new example. The changes are straightforward and localized, making the review process relatively quick and easy.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    Possible Bug: The code checks if the list of sinks is empty but does not handle the scenario where @driver.cast_sinks might return nil or an unexpected value, which could lead to errors when attempting to access sinks.first['name'].

    🔒 Security concerns

    No

    Copy link
    Contributor

    qodo-merge-pro bot commented May 8, 2024

    PR Code Suggestions ✨

    CategorySuggestions                                                                                                                                                       
    Best practice
    Add an after block to properly close the WebDriver after each test.

    To ensure that the @driver instance is properly cleaned up and closed after each test,
    it's recommended to use an after block to quit the driver. This prevents resource leakage
    and ensures that each test runs in a clean state.

    examples/ruby/spec/browsers/chrome_spec.rb [118-124]

     @driver = Selenium::WebDriver.for :chrome
     @driver.cast_sinks
     sinks = @driver.cast_sinks
     unless sinks.empty?
       device_name = sinks.first['name']
       @driver.start_cast_tab_mirroring(device_name)
       expect { @driver.stop_casting(device_name) }.not_to raise_exception
    +end
    +after(:each) do
    +  @driver.quit
    +end
     
    Specify the type of exception expected in the test for clarity.

    To improve the exception handling, specify the type of exception you expect not to be
    raised in the test. This makes the test more precise and easier to understand.

    examples/ruby/spec/browsers/chrome_spec.rb [124]

    -expect { @driver.stop_casting(device_name) }.not_to raise_exception
    +expect { @driver.stop_casting(device_name) }.not_to raise_error
     
    Performance
    Reduce redundant WebDriver calls by reusing the result stored in a variable.

    Instead of calling @driver.cast_sinks twice, store the result in a variable and reuse it.
    This reduces unnecessary calls to the WebDriver, which can improve the performance of the
    tests.

    examples/ruby/spec/browsers/chrome_spec.rb [119-120]

    -@driver.cast_sinks
     sinks = @driver.cast_sinks
     
    Maintainability
    Use a more descriptive variable name for the list of casting devices.

    To improve the readability and maintainability of the test, consider using a more
    descriptive variable name than sinks for the list of casting devices.

    examples/ruby/spec/browsers/chrome_spec.rb [120]

    -sinks = @driver.cast_sinks
    +casting_devices = @driver.cast_sinks
     
    Enhancement
    Add an assertion to ensure the list of sinks is not empty before proceeding with the test.

    To ensure that the test is more robust, add an assertion to check that the list of sinks
    is not empty before proceeding with the test. This helps in identifying issues with the
    setup or environment early.

    examples/ruby/spec/browsers/chrome_spec.rb [121-124]

    +expect(sinks).not_to be_empty
     unless sinks.empty?
       device_name = sinks.first['name']
       @driver.start_cast_tab_mirroring(device_name)
       expect { @driver.stop_casting(device_name) }.not_to raise_exception
     

    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 !

    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.

    HI @aguspe,

    Can we just include code from L119-L122 Or L124?

    @harsha509 harsha509 self-requested a review May 10, 2024 06:43
    @aguspe
    Copy link
    Contributor Author

    aguspe commented May 10, 2024

    HI @aguspe,

    Can we just include code from L119-L122 Or L124?

    Hi @harsha509 thank you so much for the question, I think the best is still L119-L124 because in the current implementation if people try just to use @driver.cast_sinks it will actually not work

    I made a question regarding this on the slack channel to see how much we needed to expand the example for the users, here is the message: https://seleniumhq.slack.com/archives/C0K0BK8MQ/p1715280896233389

    @harsha509
    Copy link
    Member

    HI @aguspe,
    Can we just include code from L119-L122 Or L124?

    Hi @harsha509 thank you so much for the question, I think the best is still L119-L124 because in the current implementation if people try just to use @driver.cast_sinks it will actually not work

    I made a question regarding this on the slack channel to see how much we needed to expand the example for the users, here is the message: https://seleniumhq.slack.com/archives/C0K0BK8MQ/p1715280896233389

    L119-L124 looks fine to me...

    @aguspe
    Copy link
    Contributor Author

    aguspe commented May 10, 2024

    HI @aguspe,
    Can we just include code from L119-L122 Or L124?

    Hi @harsha509 thank you so much for the question, I think the best is still L119-L124 because in the current implementation if people try just to use @driver.cast_sinks it will actually not work
    I made a question regarding this on the slack channel to see how much we needed to expand the example for the users, here is the message: https://seleniumhq.slack.com/archives/C0K0BK8MQ/p1715280896233389

    L119-L124 looks fine to me...

    Perfect thank you so much @harsha509, let me know if there are other questions

    @harsha509 harsha509 merged commit 7942c68 into SeleniumHQ:trunk May 11, 2024
    9 checks passed
    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