Skip to content

[rb] Adds support for the w3c silent option for the ruby library#14152

Merged
titusfortner merged 2 commits intoSeleniumHQ:trunkfrom
aguspe:rb_add_support_for_silent_option
Jun 18, 2024
Merged

[rb] Adds support for the w3c silent option for the ruby library#14152
titusfortner merged 2 commits intoSeleniumHQ:trunkfrom
aguspe:rb_add_support_for_silent_option

Conversation

@aguspe
Copy link
Contributor

@aguspe aguspe commented Jun 18, 2024 •
edited
Loading

User description

Description

While working on improving the documentation for the ruby library on this pr: SeleniumHQ/seleniumhq.github.io#1748

I noticed that the ruby library was throwing an error when the option silent was set to true for internet explorer, and that it was not implemented as it is in the js or java bindings

Motivation and Context

It's important that all the bindings support the same w3c options and that our documentation is up to day:

Current docs: https://www.selenium.dev/documentation/webdriver/browsers/internet_explorer/

Screenshot 2024-06-18 at 21 45 07

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix, Tests

Description

  • Added support for the silent option in the Internet Explorer options for the Ruby library.
  • Updated the options hash in options.rb to include the silent key.
  • Added unit tests in options_spec.rb to verify the silent option is correctly set and handled.

Changes walkthrough 📝

Relevant files
Enhancement
options.rb
Add support for `silent` option in IE options                       

rb/lib/selenium/webdriver/ie/options.rb

  • Added support for the silent option in Internet Explorer options.
  • Updated the options hash to include the silent key.
    		•  +2/-1     
    Tests
    options_spec.rb
    Add tests for `silent` option in IE options                           

    rb/spec/unit/selenium/webdriver/ie/options_spec.rb

  • Added tests for the silent option in Internet Explorer options.
  • Verified the silent option is correctly set in various test cases.
    		•  +5/-1     

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

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 18, 2024

    PR Reviewer Guide 🔍

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

    @qodo-code-review
    Copy link
    Contributor

    qodo-code-review bot commented Jun 18, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Consistency
    Prefix the new key with ie. to maintain consistency with existing keys

    To maintain consistency with the existing keys, consider prefixing the new silent key with
    ie..

    rb/lib/selenium/webdriver/ie/options.rb [46]

    -silent: 'silent'
+silent: 'ie.silent'
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion correctly identifies a consistency issue in the naming convention of the keys in the options hash. Prefixing with 'ie.' aligns with existing keys and improves code consistency.

    		8
    Test coverage
    Add a test case to verify the default value of the new option

    Add a test case to verify the default value of the silent option to ensure it behaves
    correctly when not explicitly set.

    rb/spec/unit/selenium/webdriver/ie/options_spec.rb [91]

     expect(opts.silent).to be_truthy
+opts_without_silent = Options.new
+expect(opts_without_silent.silent).to be_falsey
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a test case to verify the default behavior of the new silent option is important for ensuring expected functionality when the option is not set. This enhances test coverage and reliability.

    		7
    Add a test to verify that the new option is passed to the driver

    Ensure that the silent option is correctly passed to the underlying driver by adding a
    test that checks the final capabilities.

    rb/spec/unit/selenium/webdriver/ie/options_spec.rb [63]

     expect(opts.args.to_a).to eq(%w[foo bar])
+expect(opts.capabilities['silent']).to be(true)
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion to verify that the silent option is passed correctly to the driver is valid and improves the test suite by ensuring that the option affects the driver's behavior as expected.

    		7
    Add a test to verify that the new option can be disabled

    Consider adding a test that sets the silent option to false to ensure that the option can
    be correctly disabled.

    rb/spec/unit/selenium/webdriver/ie/options_spec.rb [91]

     expect(opts.silent).to be_truthy
+opts_with_silent_false = Options.new(silent: false)
+expect(opts_with_silent_false.silent).to be_falsey
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion is useful as it proposes testing the ability to disable the silent option, which is crucial for verifying that the option can be toggled as expected, enhancing the robustness of the test suite.

    		7

    @aguspe aguspe mentioned this pull request Jun 18, 2024
    Merged
    6 tasks
    @titusfortner titusfortner merged commit 0f7386e into SeleniumHQ:trunk Jun 18, 2024
    @seedysoft seedysoft mentioned this pull request Jun 21, 2024
    Merged
    @MiguelPelegrina MiguelPelegrina mentioned this pull request Jul 12, 2024
    Merged
    @aravindb26 aravindb26 mentioned this pull request Jul 16, 2024
    Open
    @x-way x-way mentioned this pull request Jul 24, 2024
    Closed
    @x-way x-way mentioned this pull request Aug 10, 2024
    Closed
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Oct 29, 2024
    @aguspe @sandeepsuryaprasad
    [rb] Adds support for the w3c silent option for the ruby library (Sel…
    c3999c6 
    …eniumHQ#14152)

Adds support for the w3c silent option for the ruby library

Co-authored-by: aguspe <agustin.pe94@gmail.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @titusfortner titusfortner titusfortner approved these changes

    Assignees

    No one assigned

    Labels

    P-bug fix PR addresses a known issue Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @aguspe @titusfortner