You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
It looks like an update for test fixture or something which enable downloads from the beginning, so the expectation in test not happen anymore.
Adding a step to reset the driver state with enable_downloads: false to get expect exception in test
1) Selenium::WebDriver::Remote::Driver errors when not set
Failure/Error:
expect {
driver.downloadable_files
}.to raise_exception(Error::WebDriverError,
'You must enable downloads in order to work with downloadable files.')
expected Selenium::WebDriver::Error::WebDriverError with "You must enable downloads in order to work with downloadable files.", got #<Selenium::WebDriver::Error::UnknownError: Cannot find downloads file system for session id: f94b5a8... os.arch: 'amd64', os.version: '10.0', java.version: '17.0.11'
Driver info: driver.version: unknown> with backtrace:
Motivation and Context
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)
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review
Test Modification The test case for 'errors when not set' has been modified. Verify if the new implementation correctly tests the intended behavior and if the reset_driver! method works as expected with enable_downloads: false.
reset_driver!(enable_downloads: false) do |driver|
expect {
driver.downloadable_files
}.to raise_exception(Error::WebDriverError,
'You must enable downloads in order to work with downloadable files.')
end
+reset_driver!(enable_downloads: true) do |driver|+ expect(driver.downloadable_files).to be_an(Array)+end+
Apply this suggestion
Suggestion importance[1-10]: 7
Why: Adding a test case for when downloads are enabled would enhance test coverage and ensure that the method works correctly in both scenarios. This suggestion is relevant and would improve the robustness of the test suite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VietND96
added
C-rb
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
It looks like an update for test fixture or something which enable downloads from the beginning, so the expectation in test not happen anymore.
Adding a step to reset the driver state with
enable_downloads: falseto get expect exception in testMotivation and Context
Types of changes
Checklist
PR Type
Bug fix, Tests
Description
driver_spec.rbby adding a step to reset the driver state withenable_downloads: false.WebDriverErrorwhen downloads are not enabled.PRDescriptionHeader.CHANGES_WALKTHROUGH
driver_spec.rb
Fix test by resetting driver state for download settingsrb/spec/integration/selenium/webdriver/remote/driver_spec.rb
enable_downloads: false.reset.
enabled.