Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

[rb] Migrate alert examples and add disable search engine choice #1860

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
harsha509 merged 1 commit into SeleniumHQ:trunk from aguspe:move_alert_examples
Aug 17, 2024

Conversation

@aguspe
Copy link
Contributor

@aguspe aguspe commented Aug 16, 2024 •
edited by qodo-merge-pro bot
Loading

User description

Description

This PR migrates the alert example to a spec file and adds the argument for Chrome to disable the search engine selection

Motivation and Context

The documentation must be easy to maintain, so having all the examples on the spec files helps with this

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 new tests for interacting with alerts, confirms, and prompts in Ruby.
  • Updated Ruby examples in documentation to use code references for better maintainability.
  • Enhanced the Ruby WebDriver setup by adding an option to disable the search engine choice screen in Chrome.
  • Formatted the Gemfile by adding a newline at the end.

Changes walkthrough 📝

Relevant files
Formatting
Gemfile
Add newline at end of Gemfile

examples/ruby/Gemfile

  • Added a newline at the end of the file.
+1/-1
Tests
alerts_spec.rb
Add tests for alert interactions in Ruby

examples/ruby/spec/interactions/alerts_spec.rb

  • Added tests for interacting with alerts, confirms, and prompts.
  • Included navigation to a specific URL before tests.
    • +43/-0
    Enhancement
    spec_helper.rb
    Add Chrome option to disable search engine choice

    examples/ruby/spec/spec_helper.rb

    • Added Chrome option to disable search engine choice screen.
    +3/-1
    Documentation
    alerts.en.md
    Update Ruby alert examples with code references

    website_and_docs/content/documentation/webdriver/interactions/alerts.en.md

    • Updated Ruby examples to use code references.
    +8/-38
    alerts.ja.md
    Update Ruby alert examples with code references

    website_and_docs/content/documentation/webdriver/interactions/alerts.ja.md

    • Updated Ruby examples to use code references.
    +9/-39
    alerts.pt-br.md
    Update Ruby alert examples with code references

    website_and_docs/content/documentation/webdriver/interactions/alerts.pt-br.md

    • Updated Ruby examples to use code references.
    +9/-39
    alerts.zh-cn.md
    Update Ruby alert examples with code references

    website_and_docs/content/documentation/webdriver/interactions/alerts.zh-cn.md

    • Updated Ruby examples to use code references.
    +9/-39

    💡 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 Aug 16, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 9b15025

    @aguspe aguspe marked this pull request as ready for review August 16, 2024 07:15
    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 16, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Configuration Change
    The PR adds a new Chrome option to disable the search engine choice screen. This change might affect the behavior of tests and should be reviewed for potential side effects.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion Score
    Enhancement
    Add assertions to verify the alert's text and its dismissal

    Consider adding assertions to verify the expected behavior after interacting with
    the alerts. This will ensure that the interactions are working as intended and
    improve the test's reliability.

    examples/ruby/spec/interactions/alerts_spec.rb [12-23] 

     it 'interacts with an alert' do
 driver.execute_script 'alert("Hello, World!")'
 
 # Store the alert reference in a variable
 alert = driver.switch_to.alert
 
 # Get the text of the alert
- alert.text
+ alert_text = alert.text
+ expect(alert_text).to eq('Hello, World!')
 
 # Press on Cancel button
 alert.dismiss
+
+ # Verify that the alert is no longer present
+ expect { driver.switch_to.alert }.to raise_error(Selenium::WebDriver::Error::NoSuchAlertError)
 end
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding assertions to verify the alert's text and its dismissal significantly improves the reliability and robustness of the test, ensuring that the interactions behave as expected.

    		9
    Make the Chrome options configuration more flexible by allowing additional options to be passed as parameters

    Consider using a more flexible approach for setting Chrome options by allowing
    additional options to be passed as parameters. This will make the start_session
    method more versatile for different test scenarios.

    examples/ruby/spec/spec_helper.rb [30-33] 

    -def start_session
+def start_session(additional_options = [])
 options = Selenium::WebDriver::Chrome::Options.new
 options.add_argument('disable-search-engine-choice-screen')
+ additional_options.each { |option| options.add_argument(option) }
 @driver = Selenium::WebDriver.for(:chrome, options: options)
 end
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Allowing additional options to be passed makes the start_session method more versatile and adaptable to different testing scenarios, enhancing the flexibility of the test setup.

    		8
    Best practice
    Use a more flexible version constraint for the selenium-webdriver gem to allow for minor updates

    Consider using a more flexible version constraint for the 'selenium-webdriver' gem.
    Using '~>' instead of '=' allows for minor version updates, which can include bug
    fixes and performance improvements while maintaining compatibility.

    examples/ruby/Gemfile [11] 

    -gem 'selenium-webdriver', '= 4.23.0'
+gem 'selenium-webdriver', '~> 4.23.0'
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using a more flexible version constraint allows for minor updates, which can include important bug fixes and improvements, while maintaining compatibility, thus enhancing the maintainability of the project.

    		8
    Create a helper method for triggering alerts to improve code readability and reusability

    Consider using a more descriptive method name for the JavaScript execution that
    triggers the alert. This will improve code readability and make it clearer what the
    script is doing.

    examples/ruby/spec/interactions/alerts_spec.rb [13] 

    -driver.execute_script 'alert("Hello, World!")'
+def trigger_alert(message)
+ driver.execute_script("alert(arguments[0])", message)
+end
 
+# Usage in the test
+trigger_alert('Hello, World!')
+
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using a helper method for triggering alerts improves code readability and reusability, making the test code cleaner and easier to maintain.

    		7

    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 fc556b5 into SeleniumHQ:trunk Aug 17, 2024
    selenium-ci added a commit that referenced this pull request Aug 17, 2024 
    ...)[deploy site]
Migrate alert examples and add disable search engine choice fc556b5
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @harsha509 harsha509 harsha509 approved these changes

    Assignees

    No one assigned

    Labels

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

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    AltStyle によって変換されたページ (->オリジナル) /