-
Notifications
You must be signed in to change notification settings - Fork 136
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
fix: support multiple default Chrome paths on each OS #47
base: main
Are you sure you want to change the base?
fix: support multiple default Chrome paths on each OS #47
Conversation
dannysigalovich
commented
Mar 11, 2025
- Update _get_default_binary_location to check both "Program Files" and "Program Files (x86)" directories.
- Ensure that Chrome is correctly located regardless of its installation path.
- Raise an error if Chrome is not found in the default locations.
- Update _get_default_binary_location to check both "Program Files" and "Program Files (x86)" directories. - Ensure that Chrome is correctly located regardless of its installation path. - Raise an error if Chrome is not found in the default locations.
Hi @dannysigalovich, first of all, thanks for your contribution! I'll review your PR now |
Also, please follow the Conventional Commits style; without it, I can't generate the releases. Just undo your commit and commit again with the |
This reverts commit 684db08.
- Renamed validate_browser_path() → validate_browser_paths() - Updated chrome.py to pass a list of possible paths per OS - Updated tests to use validate_browser_paths() instead of validate_browser_path()
Nice @dannysigalovich! You just have to fix the formatation now. Run |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
if path == '/first/fake/path': | ||
return False | ||
elif path == '/second/fake/path': | ||
return True | ||
return False |
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.
You can use a match
here, since the min version of Python is 3.10:
match path:
case "/first/fake/path":
return False
case "/second/fake/path":
return True
case _:
return False
_get_default_binary_location=MagicMock( | ||
return_value='/fake/path/to/browser' | ||
), | ||
Browser, |
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.
Can you remove the extra indentation to maintain the code style?