SeleniumHQ / seleniumhq.github.io

Official Selenium website and documentation
https://selenium.dev
Apache License 2.0
1.07k stars 1.27k forks source link

Add permissions examples and update the log examples #1731

Closed aguspe closed 3 months ago

aguspe commented 3 months ago

User description

Description

This PR adds examples for permissions in Chrome using Ruby, it also updates and organizes the logs test so they are under the same describe group

Motivation and Context

The documentation must be up to date so users can find concrete examples and be aware of all the possibilities that Selenium has to offer

Right now the example looks like this:

Screenshot 2024-05-20 at 17 40 55

Types of changes

Checklist


PR Type

Enhancement, Documentation


Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
chrome_spec.rb
Add and refactor tests for Chrome browser permissions and logs

examples/ruby/spec/browsers/chrome_spec.rb
  • Added a new test for setting permissions in Chrome.
  • Refactored the existing test for getting browser logs.
  • Introduced a helper method permission to check permission states.
  • +22/-7   
    Documentation
    chrome.en.md
    Update Ruby code block references in Chrome documentation (EN)

    website_and_docs/content/documentation/webdriver/browsers/chrome.en.md - Updated code block references for Ruby examples.
    +2/-2     
    chrome.ja.md
    Update Ruby code block references in Chrome documentation (JA)

    website_and_docs/content/documentation/webdriver/browsers/chrome.ja.md - Updated code block references for Ruby examples.
    +2/-2     
    chrome.pt-br.md
    Update Ruby code block references in Chrome documentation (PT-BR)

    website_and_docs/content/documentation/webdriver/browsers/chrome.pt-br.md - Updated code block references for Ruby examples.
    +2/-2     
    chrome.zh-cn.md
    Update Ruby code block references in Chrome documentation (ZH-CN)

    website_and_docs/content/documentation/webdriver/browsers/chrome.zh-cn.md - Updated code block references for Ruby examples.
    +2/-2     

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

    netlify[bot] commented 3 months ago

    Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    Latest commit 8caf46a5be2355463565942e1f17c6f54d747a3b
    codiumai-pr-agent-pro[bot] commented 3 months ago

    PR Description updated to latest commit (https://github.com/SeleniumHQ/seleniumhq.github.io/commit/8caf46a5be2355463565942e1f17c6f54d747a3b)

    codiumai-pr-agent-pro[bot] commented 3 months ago

    PR Review ๐Ÿ”

    โฑ๏ธ Estimated effort to review [1-5] 2, because the changes are mostly straightforward with the addition of new tests and minor updates to documentation. The code is well-structured and mainly involves standard API usage and test assertions.
    ๐Ÿงช Relevant tests Yes
    โšก Possible issues Possible Bug: The `permission` method uses an asynchronous script to query permissions but does not handle potential errors or rejections from the `navigator.permissions.query` method. This could lead to unhandled promise rejections if the query fails.
    ๐Ÿ”’ Security concerns No
    codiumai-pr-agent-pro[bot] commented 3 months ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Replace sleep with Selenium's wait functionality to make the test more reliable ___ **Instead of using sleep 1 to wait for the page to load, consider using Selenium's built-in
    wait functionality to wait for a specific condition, such as the presence of an element or
    a specific state.** [examples/ruby/spec/browsers/chrome_spec.rb [140-141]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1731/files#diff-f3e68237826e1b64c37885dc12602e936e6e4ea29da653aeb659d466a465ffcaR140-R141) ```diff -sleep 1 +wait = Selenium::WebDriver::Wait.new(timeout: 10) +wait.until { @driver.find_element(tag_name: 'body') } logs = @driver.logs.get(:browser) ```
    Suggestion importance[1-10]: 8 Why: Replacing `sleep` with Selenium's wait functionality improves test reliability and efficiency by waiting for specific conditions rather than a fixed time.
    8
    Add proper cleanup to ensure the browser is closed after the test ___ **Ensure that the @driver instance is properly quit after the test to avoid leaving the
    browser open.** [examples/ruby/spec/browsers/chrome_spec.rb [138-143]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1731/files#diff-f3e68237826e1b64c37885dc12602e936e6e4ea29da653aeb659d466a465ffcaR138-R143) ```diff @driver = Selenium::WebDriver.for :chrome -@driver.navigate.to 'https://www.selenium.dev/selenium/web/' -sleep 1 -logs = @driver.logs.get(:browser) -expect(logs.first.message).to include 'Failed to load resource' +begin + @driver.navigate.to 'https://www.selenium.dev/selenium/web/' + wait = Selenium::WebDriver::Wait.new(timeout: 10) + wait.until { @driver.find_element(tag_name: 'body') } + logs = @driver.logs.get(:browser) + expect(logs.first.message).to include 'Failed to load resource' +ensure + @driver.quit +end ```
    Suggestion importance[1-10]: 7 Why: Ensuring that the `@driver` instance is properly quit after the test is a best practice to avoid resource leakage, making this suggestion valuable.
    7
    Possible issue
    Add error handling to the permission method to manage potential failures in permission queries ___ **Add error handling for the permission method to manage cases where the permission query
    might fail or return unexpected results.** [examples/ruby/spec/browsers/chrome_spec.rb [166-167]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1731/files#diff-f3e68237826e1b64c37885dc12602e936e6e4ea29da653aeb659d466a465ffcaR166-R167) ```diff -@driver.execute_async_script('callback = arguments[arguments.length - 1];' \ - 'callback(navigator.permissions.query({name: arguments[0]}));', name)['state'] +result = @driver.execute_async_script('callback = arguments[arguments.length - 1];' \ + 'navigator.permissions.query({name: arguments[0]}).then(callback).catch(callback);', name) +result['state'] rescue 'unknown' ```
    Suggestion importance[1-10]: 7 Why: Adding error handling to the `permission` method is crucial for robustness, especially when dealing with asynchronous operations that might fail.
    7
    Enhancement
    Combine multiple add_permissions calls into one for conciseness ___ **Combine the two add_permissions calls into one to make the code more concise and
    efficient.** [examples/ruby/spec/browsers/chrome_spec.rb [149-150]](https://github.com/SeleniumHQ/seleniumhq.github.io/pull/1731/files#diff-f3e68237826e1b64c37885dc12602e936e6e4ea29da653aeb659d466a465ffcaR149-R150) ```diff -@driver.add_permission('camera', 'denied') -@driver.add_permissions('clipboard-read' => 'denied', 'clipboard-write' => 'prompt') +@driver.add_permissions('camera' => 'denied', 'clipboard-read' => 'denied', 'clipboard-write' => 'prompt') ```
    Suggestion importance[1-10]: 6 Why: Combining the `add_permissions` calls into one enhances code conciseness and reduces redundancy, although it's a relatively minor enhancement.
    6