-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
selenium manager: check invalid browser version #14511
Conversation
PR Reviewer Guide 🔍
|
PR Code Suggestions ✨
|
This implementation is not correct since it will make Selenium Manager to fail even for valid browser versions (e.g., Chrome 127). See for example:
Also, the error message can be enhanced a bit:
|
Oops look like I didn't consider all test cases. I will take a look 😅 |
I reviewed this PR. I'm sorry, but it is not correct. First, adding a call to
Also, the two new test cases are not adequate. First, #13418 should be done with the following changes in
|
5583c4b
to
51d082a
Compare
@bonigarcia You are right. I have committed the code changes as suggested by you. Please let me know if something is missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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
Returns an error instead of downloading the possible browser version
Motivation and Context
Fix the issue #13418
Types of changes
Checklist
PR Type
Bug fix, Tests
Description
SeleniumManager
to return an error when an invalid browser version is provided, preventing unnecessary downloads.Changes walkthrough 📝
lib.rs
Add error handling for invalid browser version
rust/src/lib.rs
browser_tests.rs
Add test for invalid browser version handling
rust/tests/browser_tests.rs