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
This file contains 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
Fixed a bug in bidi/session.py by changing the default value of the browsing_contexts parameter to None in the session_subscribe and session_unsubscribe functions to avoid using mutable objects as default values.
Added checks to initialize browsing_contexts as an empty list if it is None in the session_subscribe and session_unsubscribe functions.
Fixed linting issues in support/relative_locator.py by adjusting the formatting of @overload method definitions.
Changes walkthrough 📝
Relevant files
Bug fix
session.py
Fix mutable default argument in session_subscribe and session_unsubscribe
py/selenium/webdriver/common/bidi/session.py
Changed default value of browsing_contexts parameter to None in session_subscribe and session_unsubscribe functions.
Added checks to initialize browsing_contexts as an empty list if it is None.
Code Improvement The change from a mutable default argument to None with a conditional initialization to an empty list is a good practice to avoid potential bugs related to mutable default arguments.
Code Improvement Similar to the previous function, this change also improves the function definition by removing a mutable default argument.
Add validation for the distance parameter to ensure it is non-negative
For the near method, consider validating the distance parameter to ensure it is a non-negative integer. This validation can prevent runtime errors and enforce that the distance parameter adheres to expected values.
def near(self, element_or_locator: Union[WebElement, LocatorType], distance: int = 50) -> "RelativeBy":
+ if not isinstance(distance, int) or distance < 0:+ raise ValueError("Distance must be a non-negative integer")
Apply this suggestion
Suggestion importance[1-10]: 9
Why: Adding validation for the distance parameter is a crucial improvement that prevents potential runtime errors and ensures the parameter adheres to expected values.
9
Best practice
Use immutable default arguments where possible
To avoid potential issues with mutable default arguments, consider using a tuple for events if the function does not modify this parameter. This change ensures that the default argument is not only immutable but also safe from unintended side-effects.
Why: Using a tuple for events ensures immutability and avoids potential issues with mutable default arguments. However, it is a minor improvement since the function does not modify events.
7
Maintainability
Simplify method overloads by removing redundant default None
The above method's overload can be simplified by removing the unnecessary None default for element_or_locator in the second overload, as it does not add value and the main method already handles None as a default.
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.
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
session_subscribe
andsession_unsubscribe
, the function parameterbrowsing_contexts
had a mutable object as default value.browsing_contexts
toNone
as default value to the parameter.Motivation and Context
It is not recommended/safe to assign mutable objects as default values for function parameters.
Types of changes
Checklist
PR Type
Bug fix, Enhancement
Description
bidi/session.py
by changing the default value of thebrowsing_contexts
parameter toNone
in thesession_subscribe
andsession_unsubscribe
functions to avoid using mutable objects as default values.browsing_contexts
as an empty list if it isNone
in thesession_subscribe
andsession_unsubscribe
functions.support/relative_locator.py
by adjusting the formatting of@overload
method definitions.Changes walkthrough 📝
session.py
Fix mutable default argument in
session_subscribe
andsession_unsubscribe
py/selenium/webdriver/common/bidi/session.py
browsing_contexts
parameter toNone
insession_subscribe
andsession_unsubscribe
functions.browsing_contexts
as an empty list if it isNone
.relative_locator.py
Fix linting issues in `relative_locator.py`
py/selenium/webdriver/support/relative_locator.py
@overload
methoddefinitions.