Skip to content
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

Add all special features examples for edge using ruby #1740

Merged

Conversation

aguspe
Copy link
Contributor

@aguspe aguspe commented May 24, 2024

User description

Description

This PR adds extra tests to the edge spec file to add examples of special features in the browser, this PR is similar to #1711 and the related PRs

Motivation and Context

Is important that all of the users and community member have access to examples so they can easily start automating and working with Selenium

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

Tests, Documentation


Description

  • Added new tests for special features in the Edge browser using Ruby, including casting, network conditions, browser logs, and permissions.
  • Introduced a helper method permission to check permissions in the Edge browser.
  • Updated Edge documentation in multiple languages (EN, JA, PT-BR, ZH-CN) to link Ruby code examples to specific lines in edge_spec.rb.

Changes walkthrough 📝

Relevant files
Tests
edge_spec.rb
Add tests for special features in Edge browser using Ruby

examples/ruby/spec/browsers/edge_spec.rb

  • Added tests for casting, network conditions, browser logs, and
    permissions.
  • Introduced helper method permission to check permissions.
  • +46/-0   
    Documentation
    edge.en.md
    Update Ruby code examples links in Edge documentation (EN)

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

  • Updated Ruby code examples to link to specific lines in edge_spec.rb.
  • +4/-4     
    edge.ja.md
    Update Ruby code examples links in Edge documentation (JA)

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

  • Updated Ruby code examples to link to specific lines in edge_spec.rb.
  • +4/-4     
    edge.pt-br.md
    Update Ruby code examples links in Edge documentation (PT-BR)

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

  • Updated Ruby code examples to link to specific lines in edge_spec.rb.
  • +4/-4     
    edge.zh-cn.md
    Update Ruby code examples links in Edge documentation (ZH-CN)

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

  • Updated Ruby code examples to link to specific lines in edge_spec.rb.
  • +4/-4     

    💡 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 24, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit a18c26c

    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added documentation Improvements or additions to documentation tests Review effort [1-5]: 2 labels May 24, 2024
    Copy link
    Contributor

    PR Review 🔍

    ⏱️ Estimated effort to review [1-5]

    2, because the PR involves adding new test cases and updating documentation, which are generally straightforward to review. The changes are well-documented and localized to specific functionalities.

    🧪 Relevant tests

    Yes

    ⚡ Possible issues

    Possible Bug: The permission method in edge_spec.rb uses an asynchronous script to query permissions but does not handle the asynchronous nature in the test assertions. This might lead to flaky tests if the permissions state is not retrieved in time.

    Hardcoded Sleep: The test 'gets the browser logs' uses sleep 1 which might lead to non-deterministic behavior and could fail if the page takes longer to load. Consider using explicit waits to ensure the page and logs are fully loaded.

    🔒 Security concerns

    No

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use a before block to initialize the driver once for all tests within the describe block

    To avoid repeatedly initializing the @driver in each test case, consider using a before
    block to set up the driver once for all tests within the 'Special Features' describe
    block.

    examples/ruby/spec/browsers/edge_spec.rb [117-135]

    +before(:each) do
    +  @driver = Selenium::WebDriver.for :edge
    +end
    +
     it 'casts' do
    -  @driver = Selenium::WebDriver.for :edge
       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
     end
     ...
     it 'gets and sets network conditions' do
    -  @driver = Selenium::WebDriver.for :edge
       @driver.network_conditions = {offline: false, latency: 100, throughput: 200}
       expect(@driver.network_conditions).to eq(
         'offline' => false,
         'latency' => 100,
         'download_throughput' => 200,
         'upload_throughput' => 200)
     end
     
    Suggestion importance[1-10]: 8

    Why: This suggestion correctly identifies a repeated pattern of initializing @driver in each test case and proposes a more efficient setup using a before block, which is a best practice in test automation for reducing redundancy and improving test execution time.

    8
    Add an after block to quit the driver after each test to avoid resource leaks

    Ensure the @driver is properly quit after each test to avoid resource leaks by adding an
    after block to quit the driver.

    examples/ruby/spec/browsers/edge_spec.rb [116-155]

     describe 'Special Features' do
    +  before(:each) do
    +    @driver = Selenium::WebDriver.for :edge
    +  end
    +
    +  after(:each) do
    +    @driver.quit
    +  end
    +
       it 'casts' do
    -    @driver = Selenium::WebDriver.for :edge
         ...
       end
       ...
     end
     
    Suggestion importance[1-10]: 8

    Why: The suggestion to add an after block to quit the driver is crucial for resource management and avoiding leaks, which is particularly important in automated testing environments. This is a best practice for maintaining clean test states.

    8
    Performance
    Replace sleep with WebDriverWait to ensure the page has loaded before retrieving logs

    Replace the sleep 1 statement with a more reliable wait mechanism, such as WebDriverWait,
    to ensure the page has loaded before retrieving logs.

    examples/ruby/spec/browsers/edge_spec.rb [139-141]

     @driver.navigate.to 'https://www.selenium.dev/selenium/web/'
    -sleep 1
    +wait = Selenium::WebDriver::Wait.new(timeout: 10)
    +wait.until { @driver.execute_script('return document.readyState') == 'complete' }
     logs = @driver.logs.get(:browser)
     
    Suggestion importance[1-10]: 7

    Why: Replacing sleep with WebDriverWait is a valid improvement for making the test more reliable by ensuring the page is fully loaded. This change enhances the robustness of the test.

    7
    Possible issue
    Add error handling for the @driver.cast_sinks method to manage potential exceptions

    Add error handling for the @driver.cast_sinks method to manage potential exceptions that
    might occur when fetching cast sinks.

    examples/ruby/spec/browsers/edge_spec.rb [119-124]

    -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
    +begin
    +  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
    +rescue StandardError => e
    +  puts "Error fetching cast sinks: #{e.message}"
     end
     
    Suggestion importance[1-10]: 6

    Why: Adding error handling for potential exceptions from @driver.cast_sinks is a good practice to prevent test failures from unhandled errors. This suggestion improves the resilience of the test code.

    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 tou @aguspe !

    @harsha509 harsha509 merged commit c56ec04 into SeleniumHQ:trunk May 25, 2024
    9 checks passed
    selenium-ci added a commit that referenced this pull request May 25, 2024
    …site]
    
    Co-authored-by: aguspe <agustin.pe94@gmail.com> c56ec04
    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 Review effort [1-5]: 2 tests
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants