[py] Refactored `server.py` in a more pythonic approach. by sandeepsuryaprasad · Pull Request #15840 · SeleniumHQ/selenium
PR Reviewer Guide 🔍
Here are some key observations to aid the review process:
|
🎫 Ticket compliance analysis ❌
1234 - Not compliant
Non-compliant requirements:
• Fix issue where Selenium 2.48 doesn't trigger JavaScript in link's href on click()
• Ensure JavaScript in href attributes works properly when clicked in Firefox 42.0
5678 - Not compliant
Non-compliant requirements:
• Fix "ConnectFailure (Connection refused)" error when instantiating ChromeDriver
• Address issue where subsequent ChromeDriver instantiations fail after the first one succeeds
|
| ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ |
| 🧪 No relevant tests |
| 🔒 No security concerns identified |
⚡ Recommended focus areas for reviewClass Reference
The __class__.__name__ reference in error messages is used without proper context. This should be replaced with the actual class name or self.class.name to ensure correct error messages.
raise TypeError(f"{__class__.__name__}.__init__() got an invalid port: '{port}'")
if not (0 <= port <= 65535):Missing Docstrings
The newly added properties lack docstrings explaining their purpose and usage, which reduces code maintainability and makes it harder for other developers to understand the API.
@property
def status_url(self):
host = self.host if self.host is not None else "localhost"
return f"http://{host}:{self.port}/status"
@property
def path(self):
return self._path
@path.setter
def path(self, path):
if path and not os.path.exists(path):
raise OSError(f"Can't find server .jar located at {path}")
self._path = path
@property
def port(self):
return self._port
@port.setter
def port(self, port):
try:
port = int(port)
except ValueError:
raise TypeError(f"{__class__.__name__}.__init__() got an invalid port: '{port}'")
if not (0 <= port <= 65535):
raise ValueError("port must be 0-65535")
self._port = port
@property
def version(self):
return self._version
@version.setter
def version(self, version):
if version:
if not re.match(r"^\d+\.\d+\.\d+$", str(version)):
raise TypeError(f"{__class__.__name__}.__init__() got an invalid version: '{version}'")
self._version = version
@property
def log_level(self):
return self._log_level
@log_level.setter
def log_level(self, log_level):
levels = ("SEVERE", "WARNING", "INFO", "CONFIG", "FINE", "FINER", "FINEST")
if log_level not in levels:
raise TypeError(f"log_level must be one of: {', '.join(levels)}")
self._log_level = log_level
@property
def env(self):
return self._env
@env.setter
def env(self, env):
if env is not None and not isinstance(env, collections.abc.Mapping):
raise TypeError("env must be a mapping of environment variables")
self._env = env |