Skip to content

Commit

Permalink
pylint fixes including tidy of f strings, simplifications of conditio…
Browse files Browse the repository at this point in the history
…nal statements and isinstances (SeleniumHQ#11205)

pylint fixes

Tidying up of a bunch of pylint issues

Co-authored-by: Simon K <jackofspaces@gmail.com>
  • Loading branch information
marksmayo and symonk committed Jan 24, 2023
1 parent 484359c commit 3f6717d
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 65 deletions.
54 changes: 25 additions & 29 deletions py/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,11 @@ def escape_backticks(docstr):
def replace_one(match):
if match.group(2) == 's':
return f"``{match.group(1)}``'s"
elif match.group(2):
if match.group(2):
# This case (some trailer other than "s") doesn't currently exist
# in the CDP definitions, but it's here just to be safe.
return f'``{match.group(1)}`` {match.group(2)}'
else:
return f'``{match.group(1)}``'
return f'``{match.group(1)}``'

# Sometimes pipes are used where backticks should have been used.
docstr = docstr.replace('|', '`')
Expand Down Expand Up @@ -179,17 +178,15 @@ def get_annotation(cls, cdp_type):
''' Return a type annotation for the CDP type. '''
if cdp_type == 'any':
return 'typing.Any'
else:
return cls[cdp_type].value
return cls[cdp_type].value

@classmethod
def get_constructor(cls, cdp_type, val):
''' Return the code to construct a value for a given CDP type. '''
if cdp_type == 'any':
return val
else:
cons = cls[cdp_type].value
return f'{cons}({val})'
cons = cls[cdp_type].value
return f'{cons}({val})'


@dataclass
Expand Down Expand Up @@ -333,18 +330,17 @@ def from_json(cls, type_):
type_['type'],
CdpItems.from_json(type_['items']) if 'items' in type_ else None,
type_.get('enum'),
[CdpProperty.from_json(p) for p in type_.get('properties', list())],
[CdpProperty.from_json(p) for p in type_.get('properties', [])],
)

def generate_code(self):
''' Generate Python code for this type. '''
logger.debug('Generating type %s: %s', self.id, self.type)
if self.enum:
return self.generate_enum_code()
elif self.properties:
if self.properties:
return self.generate_class_code()
else:
return self.generate_primitive_code()
return self.generate_primitive_code()

def generate_primitive_code(self):
''' Generate code for a primitive type. '''
Expand Down Expand Up @@ -395,7 +391,7 @@ def generate_enum_code(self):
def to_json(self):
return self.value''')

def_from_json = dedent(f'''\
def_from_json = dedent('''\
@classmethod
def from_json(cls, json):
return cls(json)''')
Expand Down Expand Up @@ -449,12 +445,12 @@ def to_json(self):

# Emit from_json() method. The properties are sorted in the same order
# as above for readability.
def_from_json = dedent(f'''\
def_from_json = dedent('''\
@classmethod
def from_json(cls, json):
return cls(
''')
from_jsons = list()
from_jsons = []
for p in props:
from_json = p.generate_from_json(dict_='json')
from_jsons.append(f'{p.py_name}={from_json},')
Expand Down Expand Up @@ -526,10 +522,10 @@ def generate_doc(self):
doc = f':param {self.py_name}:'

if self.experimental:
doc += f' **(EXPERIMENTAL)**'
doc += ' **(EXPERIMENTAL)**'

if self.optional:
doc += f' *(Optional)*'
doc += ' *(Optional)*'

if self.description:
desc = self.description.replace('`', '``').replace('\n', ' ')
Expand Down Expand Up @@ -600,8 +596,8 @@ def py_name(self):
@classmethod
def from_json(cls, command, domain) -> 'CdpCommand':
''' Instantiate a CDP command from a JSON object. '''
parameters = command.get('parameters', list())
returns = command.get('returns', list())
parameters = command.get('parameters', [])
returns = command.get('returns', [])

return cls(
command['name'],
Expand Down Expand Up @@ -653,7 +649,7 @@ def generate_code(self):
if self.description:
doc = self.description
if self.experimental:
doc += f'\n\n**EXPERIMENTAL**'
doc += '\n\n**EXPERIMENTAL**'
if self.parameters and doc:
doc += '\n\n'
elif not self.parameters and self.returns:
Expand Down Expand Up @@ -735,7 +731,7 @@ def from_json(cls, json: dict, domain: str):
json.get('deprecated', False),
json.get('experimental', False),
[typing.cast(CdpParameter, CdpParameter.from_json(p))
for p in json.get('parameters', list())],
for p in json.get('parameters', [])],
domain
)

Expand Down Expand Up @@ -804,16 +800,16 @@ def module(self):
@classmethod
def from_json(cls, domain: dict):
''' Instantiate a CDP domain from a JSON object. '''
types = domain.get('types', list())
commands = domain.get('commands', list())
events = domain.get('events', list())
types = domain.get('types', [])
commands = domain.get('commands', [])
events = domain.get('events', [])
domain_name = domain['domain']

return cls(
domain_name,
domain.get('description'),
domain.get('experimental', False),
domain.get('dependencies', list()),
domain.get('dependencies', []),
[CdpType.from_json(type) for type in types],
[CdpCommand.from_json(command, domain_name)
for command in commands],
Expand Down Expand Up @@ -939,12 +935,12 @@ def parse(json_path, output_path):
:returns: a list of CDP domain objects
'''
global current_version
with open(json_path) as json_file:
with open(json_path, encoding="utf-8") as json_file:
schema = json.load(json_file)
version = schema['version']
assert (version['major'], version['minor']) == ('1', '3')
current_version = f'{version["major"]}.{version["minor"]}'
domains = list()
domains = []
for domain in schema['domains']:
domains.append(CdpDomain.from_json(domain))
return domains
Expand All @@ -957,7 +953,7 @@ def generate_init(init_path, domains):
:param list[tuple] modules: a list of modules each represented as tuples
of (name, list_of_exported_symbols)
'''
with open(init_path, "w") as init_file:
with open(init_path, "w", encoding="utf-8") as init_file:
init_file.write(INIT_HEADER)
for domain in domains:
init_file.write(f'from . import {domain.module}\n')
Expand Down Expand Up @@ -1001,7 +997,7 @@ def main(browser_protocol_path, js_protocol_path, output_path):
subpath.unlink()

# Parse domains
domains = list()
domains = []
for json_path in json_paths:
logger.info('Parsing JSON file %s', json_path)
domains.extend(parse(json_path, output_path))
Expand Down
2 changes: 1 addition & 1 deletion py/selenium/webdriver/chromium/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def __init__(
DeprecationWarning,
stacklevel=2,
)
if keep_alive != DEFAULT_KEEP_ALIVE and type(self) == __class__:
if keep_alive != DEFAULT_KEEP_ALIVE and isinstance(self, __class__):
warnings.warn(
"keep_alive has been deprecated, please pass in a Service object", DeprecationWarning, stacklevel=2
)
Expand Down
6 changes: 3 additions & 3 deletions py/selenium/webdriver/common/bidi/cdp.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ def __init__(self, ws, session_id, target_id):
self.target_id = target_id
self.channels = defaultdict(set)
self.id_iter = itertools.count()
self.inflight_cmd = dict()
self.inflight_result = dict()
self.inflight_cmd = {}
self.inflight_result = {}

async def execute(self, cmd: typing.Generator[dict, T, typing.Any]) -> T:
"""Execute a command on the server and wait for the result.
Expand Down Expand Up @@ -384,7 +384,7 @@ def __init__(self, ws):
:param trio_websocket.WebSocketConnection ws:
"""
super().__init__(ws, session_id=None, target_id=None)
self.sessions = dict()
self.sessions = {}

async def aclose(self):
"""Close the underlying WebSocket connection.
Expand Down
5 changes: 2 additions & 3 deletions py/selenium/webdriver/common/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,11 @@ def _start_process(self, path: str) -> None:
raise WebDriverException(
f"'{os.path.basename(self.path)}' executable needs to be in PATH. {self.start_error_message}"
)
elif err.errno == errno.EACCES:
if err.errno == errno.EACCES:
raise WebDriverException(
f"'{os.path.basename(self.path)}' executable may have wrong permissions. {self.start_error_message}"
)
else:
raise
raise
except Exception as e:
raise WebDriverException(
f"The executable {os.path.basename(self.path)} needs to be available in the path. {self.start_error_message}\n{str(e)}"
Expand Down
2 changes: 1 addition & 1 deletion py/selenium/webdriver/remote/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ def get_credentials(self) -> List[Credential]:
def remove_credential(self, credential_id: Union[str, bytearray]) -> None:
"""Removes a credential from the authenticator."""
# Check if the credential is bytearray converted to b64 string
if type(credential_id) is bytearray:
if isinstance(credential_id, bytearray):
credential_id = urlsafe_b64encode(credential_id).decode()

self.execute(
Expand Down
2 changes: 1 addition & 1 deletion py/selenium/webdriver/remote/webelement.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def shadow_root(self) -> ShadowRoot:
"safari",
], "This only currently works in Chromium based browsers"
assert (
not browser_main_version <= 95
browser_main_version > 95
), f"Please use Chromium based browsers with version 96 or later. Version used {self._parent.caps['browserVersion']}"
return self._execute(Command.GET_SHADOW_ROOT)["value"]

Expand Down
2 changes: 1 addition & 1 deletion py/test/runner/run_pytest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import pytest

with open("pytest.ini", "w") as ini_file:
with open("pytest.ini", "w", encoding="utf-8") as ini_file:
ini_file.write("[pytest]\n")
ini_file.write("addopts = -r=a\n")
ini_file.write("rootdir = py\n")
Expand Down
2 changes: 1 addition & 1 deletion py/test/selenium/webdriver/chrome/proxy_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def test_bad_proxy_doesnt_interfere():

assert hasattr(driver, "command_executor")
assert hasattr(driver.command_executor, "_proxy_url")
assert type(driver.command_executor._conn) == urllib3.PoolManager
assert isinstance(driver.command_executor._conn, urllib3.PoolManager)
os.environ.pop("https_proxy")
os.environ.pop("http_proxy")
driver.quit()
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def reset_timeouts(driver):
def test_should_not_timeout_if_callback_invoked_immediately(driver, pages):
pages.load("ajaxy_page.html")
result = driver.execute_async_script("arguments[arguments.length - 1](123);")
assert type(result) == int
assert isinstance(result, int)
assert 123 == result


Expand All @@ -55,15 +55,15 @@ def test_should_be_able_to_return_an_array_literal_from_an_async_script(driver,
pages.load("ajaxy_page.html")
result = driver.execute_async_script("arguments[arguments.length - 1]([]);")
assert "Expected not to be null!", result is not None
assert type(result) == list
assert isinstance(result, list)
assert len(result) == 0


def test_should_be_able_to_return_an_array_object_from_an_async_script(driver, pages):
pages.load("ajaxy_page.html")
result = driver.execute_async_script("arguments[arguments.length - 1](new Array());")
assert "Expected not to be null!", result is not None
assert type(result) == list
assert isinstance(result, list)
assert len(result) == 0


Expand All @@ -73,7 +73,7 @@ def test_should_be_able_to_return_arrays_of_primitives_from_async_scripts(driver
result = driver.execute_async_script("arguments[arguments.length - 1]([null, 123, 'abc', true, false]);")

assert result is not None
assert type(result) == list
assert isinstance(result, list)
assert not bool(result.pop())
assert bool(result.pop())
assert "abc" == result.pop()
Expand All @@ -96,7 +96,7 @@ def test_should_be_able_to_return_arrays_of_web_elements_from_async_scripts(driv

result = driver.execute_async_script("arguments[arguments.length - 1]([document.body, document.body]);")
assert result is not None
assert type(result) == list
assert isinstance(result, list)

list_ = result
assert 2 == len(list_)
Expand Down
12 changes: 6 additions & 6 deletions py/test/selenium/webdriver/common/executing_javascript_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ def test_should_be_able_to_execute_simple_javascript_and_return_astring(driver,

result = driver.execute_script("return document.title")

assert type(result) == str, "The type of the result is %s" % type(result)
assert isinstance(result, str), "The type of the result is %s" % type(result)
assert "XHTML Test Page" == result


def test_should_be_able_to_execute_simple_javascript_and_return_an_integer(driver, pages):
pages.load("nestedElements.html")
result = driver.execute_script("return document.getElementsByName('checky').length")

assert type(result) == int
assert isinstance(result, int)
assert int(result) > 1


Expand Down Expand Up @@ -124,7 +124,7 @@ def test_should_be_able_to_execute_simple_javascript_and_return_aboolean(driver,
result = driver.execute_script("return true")

assert result is not None
assert type(result) == bool
assert isinstance(result, bool)
assert bool(result)


Expand All @@ -149,23 +149,23 @@ def test_should_be_able_to_execute_simple_javascript_and_return_an_array(driver,
expectedResult.append(subList)
result = driver.execute_script("return ['zero', [true, false]]")
assert result is not None
assert type(result) == list
assert isinstance(result, list)
assert expectedResult == result


def test_passing_and_returning_an_int_should_return_awhole_number(driver, pages):
pages.load("javascriptPage.html")
expectedResult = 1
result = driver.execute_script("return arguments[0]", expectedResult)
assert type(result) == int
assert isinstance(result, int)
assert expectedResult == result


def test_passing_and_returning_adouble_should_return_adecimal(driver, pages):
pages.load("javascriptPage.html")
expectedResult = 1.2
result = driver.execute_script("return arguments[0]", expectedResult)
assert type(result) == float
assert isinstance(result, float)
assert expectedResult == result


Expand Down
4 changes: 2 additions & 2 deletions py/test/selenium/webdriver/marionette/mn_options_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class TestUnit:
def test_ctor(self):
opts = Options()
assert opts._binary is None
assert opts._preferences == {}
assert not opts._preferences
assert opts._profile is None
assert opts._arguments == []
assert not opts._arguments
assert isinstance(opts.log, Log)

def test_binary(self):
Expand Down
Loading

0 comments on commit 3f6717d

Please sign in to comment.