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

aguspe commented 1 week ago

User description

Description

While working on improving the documentation for the ruby library on this pr: https://github.com/SeleniumHQ/seleniumhq.github.io/pull/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/

Types of changes

[x] 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

[x] I have read the contributing document.
[x] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.
[x] I have added tests to cover my changes.
[x] 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

codiumai-pr-agent-pro[bot] commented 1 week ago

PR Reviewer Guide 🔍

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

codiumai-pr-agent-pro[bot] commented 1 week ago

PR Code Suggestions ✨

Category	Suggestion	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]](https://github.com/SeleniumHQ/selenium/pull/14152/files#diff-71783770e151a311faf3e3c8e3b97694333389a75156cb1997a657c88f29f629R46-R46) ```diff -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]](https://github.com/SeleniumHQ/selenium/pull/14152/files#diff-3b858bb49ee7601f248096afaceabd682445cffa2babe6026395f13d6be83b81R91-R91) ```diff 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]](https://github.com/SeleniumHQ/selenium/pull/14152/files#diff-3b858bb49ee7601f248096afaceabd682445cffa2babe6026395f13d6be83b81R63-R63) ```diff 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]](https://github.com/SeleniumHQ/selenium/pull/14152/files#diff-3b858bb49ee7601f248096afaceabd682445cffa2babe6026395f13d6be83b81R91-R91) ```diff 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

SeleniumHQ / selenium