[py][bidi]: add `timestamp` to `HistoryUpdatedParams` class by navin772 · Pull Request #15892 · SeleniumHQ/selenium
PR Reviewer Guide 🔍
Here are some key observations to aid the review process:
|
🎫 Ticket compliance analysis ❌
5678 - Not compliant
Non-compliant requirements:
• Fix ChromeDriver connection failure error that occurs after first instance
• Resolve "Error: ConnectFailure (Connection refused)" for subsequent ChromeDriver instances
• Ensure proper ChromeDriver instantiation on Ubuntu 16.04.4 with Chrome 65.0.3325.181
1234 - Not compliant
Non-compliant requirements:
• Fix JavaScript execution in link href on click() method
• Ensure JavaScript alerts are triggered when clicking links with JavaScript in href
• Restore functionality that worked in version 2.47.1 but broke in 2.48.0/2.48.2
• Fix compatibility with Firefox 42.0
|
| ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ |
| 🧪 No relevant tests |
| 🔒 No security concerns identified |
⚡ Recommended focus areas for reviewMissing Tests
The addition of a new required parameter to the constructor and validation logic lacks corresponding unit tests to verify the timestamp validation behavior and proper instantiation.
def __init__(
self,
context: str,
timestamp: int,
url: str,
):
self.context = context
self.timestamp = timestamp
self.url = url
@classmethod
def from_json(cls, json: dict) -> "HistoryUpdatedParams":
"""Creates a HistoryUpdatedParams instance from a dictionary.
Parameters:
-----------
json: A dictionary containing the history updated parameters.
Returns:
-------
HistoryUpdatedParams: A new instance of HistoryUpdatedParams.
"""
context = json.get("context")
if context is None or not isinstance(context, str):
raise ValueError("context is required and must be a string")
timestamp = json.get("timestamp")
if timestamp is None or not isinstance(timestamp, int) or timestamp < 0:
raise ValueError("timestamp is required and must be a non-negative integer")
url = json.get("url")
if url is None or not isinstance(url, str):
raise ValueError("url is required and must be a string")
return cls(
context=context,
timestamp=timestamp,
url=url,
) |