From 4c1b6f549404826a75cfb3326cc0352cdc93b27e Mon Sep 17 00:00:00 2001 From: Dan Fandrich Date: Thu, 2 Apr 2026 17:49:37 -0700 Subject: [PATCH] tests: enable more ruff checks - Checks for missing explicit `return` statements at the end of functions that can return non-`None` values. - Checks for classes that inherit from `object`. - Checks for useless expressions. - Within an `except*` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling - Checks for variable assignments that immediately precede a `return` of the assigned variable. - Checks for `else` statements with a `return` statement in the preceding `if` block. - Checks for unnecessary parentheses on raised exceptions. Closes: #21258 --- scripts/pythonlint.sh | 5 ++++- tests/dictserver.py | 2 +- tests/http/conftest.py | 7 +++--- tests/http/scorecard.py | 37 +++++++++++++------------------ tests/http/test_17_ssl_use.py | 2 +- tests/http/test_20_websockets.py | 4 ++-- tests/http/test_30_vsftpd.py | 2 +- tests/http/test_31_vsftpds.py | 2 +- tests/http/test_32_ftps_vsftpd.py | 2 +- tests/http/testenv/caddy.py | 4 ++-- tests/http/testenv/certs.py | 11 +++++---- tests/http/testenv/client.py | 6 ++--- tests/http/testenv/curl.py | 12 +++++----- tests/http/testenv/dante.py | 4 ++-- tests/http/testenv/env.py | 15 +++++-------- tests/http/testenv/httpd.py | 15 ++++++------- tests/http/testenv/nghttpx.py | 4 ++-- tests/http/testenv/sshd.py | 4 ++-- tests/http/testenv/vsftpd.py | 4 ++-- tests/negtelnetserver.py | 6 ++--- tests/smbserver.py | 6 ++--- tests/util.py | 2 +- 22 files changed, 72 insertions(+), 84 deletions(-) diff --git a/scripts/pythonlint.sh b/scripts/pythonlint.sh index b7dc95633b..fcebd100db 100755 --- a/scripts/pythonlint.sh +++ b/scripts/pythonlint.sh @@ -27,4 +27,7 @@ # locations, or all Python files found in the current directory tree by # default. -ruff check --extend-select=B007,B016,C405,C416,COM818,D200,D213,D204,D401,D415,FURB129,N818,PERF401,PERF403,PIE790,PIE808,PLW0127,Q004,RUF010,SIM101,SIM117,SIM118,TRY400,TRY401 "$@" +ruff check --extend-select=B007,B016,C405,C416,COM818,D200,D213,D204,D401,\ +D415,FURB129,N818,PERF401,PERF403,PIE790,PIE808,PLW0127,Q004,RUF010,SIM101,\ +SIM117,SIM118,TRY400,TRY401,RET503,RET504,UP004,B018,B904,RSE102,RET505 \ +"$@" diff --git a/tests/dictserver.py b/tests/dictserver.py index 41143d6f71..2c608c2792 100755 --- a/tests/dictserver.py +++ b/tests/dictserver.py @@ -157,7 +157,7 @@ def setup_logging(options): root_logger.addHandler(stdout_handler) -class ScriptRC(object): +class ScriptRC: """Enum for script return codes.""" SUCCESS = 0 diff --git a/tests/http/conftest.py b/tests/http/conftest.py index 79ed3eacca..3715e8c7bc 100644 --- a/tests/http/conftest.py +++ b/tests/http/conftest.py @@ -75,10 +75,9 @@ def pytest_report_header(config): @pytest.fixture(scope='session') def env_config(pytestconfig, testrun_uid, worker_id) -> EnvConfig: - env_config = EnvConfig(pytestconfig=pytestconfig, - testrun_uid=testrun_uid, - worker_id=worker_id) - return env_config + return EnvConfig(pytestconfig=pytestconfig, + testrun_uid=testrun_uid, + worker_id=worker_id) @pytest.fixture(scope='session', autouse=True) diff --git a/tests/http/scorecard.py b/tests/http/scorecard.py index 6fe6d3c9ba..573923380b 100644 --- a/tests/http/scorecard.py +++ b/tests/http/scorecard.py @@ -52,12 +52,11 @@ class Card: def fmt_size(cls, val): if val >= (1024*1024*1024): return f'{val / (1024*1024*1024):0.000f}GB' - elif val >= (1024 * 1024): + if val >= (1024 * 1024): return f'{val / (1024*1024):0.000f}MB' - elif val >= 1024: + if val >= 1024: return f'{val / 1024:0.000f}KB' - else: - return f'{val:0.000f}B' + return f'{val:0.000f}B' @classmethod def fmt_mbs(cls, val): @@ -65,10 +64,9 @@ class Card: return '--' if val >= (1024*1024): return f'{val/(1024*1024):.3g} MB/s' - elif val >= 1024: + if val >= 1024: return f'{val / 1024:.3g} KB/s' - else: - return f'{val:.3g} B/s' + return f'{val:.3g} B/s' @classmethod def fmt_speed(cls, val): @@ -76,10 +74,9 @@ class Card: return '--' if val >= (10*1024*1024): return f'{(val/(1024*1024)):.3f} MB/s' - elif val >= (10*1024): + if val >= (10*1024): return f'{val/1024:.3f} KB/s' - else: - return f'{val:.3f} B/s' + return f'{val:.3f} B/s' @classmethod def fmt_speed_result(cls, val, limit): @@ -88,10 +85,9 @@ class Card: pct = ((val / limit) * 100) - 100 if val >= (10*1024*1024): return f'{(val/(1024*1024)):.3f} MB/s, {pct:+.1f}%' - elif val >= (10*1024): + if val >= (10*1024): return f'{val/1024:.3f} KB/s, {pct:+.1f}%' - else: - return f'{val:.3f} B/s, {pct:+.1f}%' + return f'{val:.3f} B/s, {pct:+.1f}%' @classmethod def fmt_reqs(cls, val): @@ -255,11 +251,11 @@ class ScoreRunner: raise Exception(f'unrecognised limit-rate: {self._limit_rate}') self._limit_rate_num = float(m.group(1)) if m.group(3) == 'g': - self._limit_rate_num *= (1024*1024*1024) + self._limit_rate_num *= 1024*1024*1024 elif m.group(3) == 'm': - self._limit_rate_num *= (1024*1024) + self._limit_rate_num *= 1024*1024 elif m.group(3) == 'k': - self._limit_rate_num *= (1024) + self._limit_rate_num *= 1024 elif m.group(3) == 'b': pass else: @@ -368,8 +364,7 @@ class ScoreRunner: profiles.append(r.profile) if self._limit_rate: return Card.mk_speed_cell(samples, profiles, errors, self._limit_rate_num) - else: - return Card.mk_mbs_cell(samples, profiles, errors) + return Card.mk_mbs_cell(samples, profiles, errors) def dl_serial(self, url: str, count: int, nsamples: int = 1): samples = [] @@ -397,8 +392,7 @@ class ScoreRunner: profiles.append(r.profile) if self._limit_rate: return Card.mk_speed_cell(samples, profiles, errors, self._limit_rate_num) - else: - return Card.mk_mbs_cell(samples, profiles, errors) + return Card.mk_mbs_cell(samples, profiles, errors) def dl_parallel(self, url: str, count: int, nsamples: int = 1): samples = [] @@ -431,8 +425,7 @@ class ScoreRunner: profiles.append(r.profile) if self._limit_rate: return Card.mk_speed_cell(samples, profiles, errors, self._limit_rate_num) - else: - return Card.mk_mbs_cell(samples, profiles, errors) + return Card.mk_mbs_cell(samples, profiles, errors) def downloads(self, count: int, fsizes: List[int], meta: Dict[str, Any]) -> Dict[str, Any]: nsamples = meta['samples'] diff --git a/tests/http/test_17_ssl_use.py b/tests/http/test_17_ssl_use.py index 34a8a2d394..de076c4ad5 100644 --- a/tests/http/test_17_ssl_use.py +++ b/tests/http/test_17_ssl_use.py @@ -466,7 +466,7 @@ class TestSSLUse: # clean session file first, then reuse session_file = os.path.join(env.gen_dir, 'test_17_15.sessions') if os.path.exists(session_file): - return os.remove(session_file) + os.remove(session_file) xargs = ['--tls-max', '1.3', '--tlsv1.3', '--ssl-sessions', session_file] curl = CurlClient(env=env, run_env=run_env) # tell the server to close the connection after each request diff --git a/tests/http/test_20_websockets.py b/tests/http/test_20_websockets.py index 2612b0afe1..979a7e5db9 100644 --- a/tests/http/test_20_websockets.py +++ b/tests/http/test_20_websockets.py @@ -62,11 +62,11 @@ class TestWebsockets: def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _rmrf(self, path): if os.path.exists(path): - return shutil.rmtree(path) + shutil.rmtree(path) @pytest.fixture(autouse=True, scope='class') def ws_echo(self, env): diff --git a/tests/http/test_30_vsftpd.py b/tests/http/test_30_vsftpd.py index bcc8b76fdd..194f3db6f8 100644 --- a/tests/http/test_30_vsftpd.py +++ b/tests/http/test_30_vsftpd.py @@ -143,7 +143,7 @@ class TestVsFTPD: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) # check with `tcpdump` if curl causes any TCP RST packets @pytest.mark.skipif(condition=not Env.tcpdump(), reason="tcpdump not available") diff --git a/tests/http/test_31_vsftpds.py b/tests/http/test_31_vsftpds.py index ba4696c0cb..a16e6b9fd0 100644 --- a/tests/http/test_31_vsftpds.py +++ b/tests/http/test_31_vsftpds.py @@ -148,7 +148,7 @@ class TestVsFTPD: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) # check with `tcpdump` if curl causes any TCP RST packets @pytest.mark.skipif(condition=not Env.tcpdump(), reason="tcpdump not available") diff --git a/tests/http/test_32_ftps_vsftpd.py b/tests/http/test_32_ftps_vsftpd.py index bac40580ee..2010daec22 100644 --- a/tests/http/test_32_ftps_vsftpd.py +++ b/tests/http/test_32_ftps_vsftpd.py @@ -161,7 +161,7 @@ class TestFtpsVsFTPD: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) # check with `tcpdump` if curl causes any TCP RST packets @pytest.mark.skipif(condition=not Env.tcpdump(), reason="tcpdump not available") diff --git a/tests/http/testenv/caddy.py b/tests/http/testenv/caddy.py index d7386bc28a..8805e88c2f 100644 --- a/tests/http/testenv/caddy.py +++ b/tests/http/testenv/caddy.py @@ -156,11 +156,11 @@ class Caddy: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): domain1 = self.env.domain1 diff --git a/tests/http/testenv/certs.py b/tests/http/testenv/certs.py index c9a30aaac0..9848590f05 100644 --- a/tests/http/testenv/certs.py +++ b/tests/http/testenv/certs.py @@ -101,7 +101,7 @@ class CertificateSpec: def name(self) -> Optional[str]: if self._name: return self._name - elif self.domains: + if self.domains: return self.domains[0] return None @@ -109,9 +109,9 @@ class CertificateSpec: def type(self) -> Optional[str]: if self.domains and len(self.domains): return "server" - elif self.client: + if self.client: return "client" - elif self.name: + if self.name: return "ca" return None @@ -144,10 +144,9 @@ class Credentials: def key_type(self): if isinstance(self._pkey, RSAPrivateKey): return f"rsa{self._pkey.key_size}" - elif isinstance(self._pkey, EllipticCurvePrivateKey): + if isinstance(self._pkey, EllipticCurvePrivateKey): return f"{self._pkey.curve.name}" - else: - raise Exception(f"unknown key type: {self._pkey}") + raise Exception(f"unknown key type: {self._pkey}") @property def private_key(self) -> Any: diff --git a/tests/http/testenv/client.py b/tests/http/testenv/client.py index e3806de0da..7f22155c10 100644 --- a/tests/http/testenv/client.py +++ b/tests/http/testenv/client.py @@ -71,15 +71,15 @@ class LocalClient: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _rmrf(self, path): if os.path.exists(path): - return shutil.rmtree(path) + shutil.rmtree(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def run(self, args): self._rmf(self._stdoutfile) diff --git a/tests/http/testenv/curl.py b/tests/http/testenv/curl.py index 4528048488..3d443700c0 100644 --- a/tests/http/testenv/curl.py +++ b/tests/http/testenv/curl.py @@ -673,15 +673,15 @@ class CurlClient: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _rmrf(self, path): if os.path.exists(path): - return shutil.rmtree(path) + shutil.rmtree(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def get_proxy_args(self, proto: str = 'http/1.1', proxys: bool = True, tunnel: bool = False, @@ -1045,10 +1045,10 @@ class CurlClient: try: p.wait(timeout=ptimeout) break - except subprocess.TimeoutExpired: + except subprocess.TimeoutExpired as e: if end_at and datetime.now() >= end_at: p.kill() - raise subprocess.TimeoutExpired(cmd=args, timeout=self._timeout) + raise subprocess.TimeoutExpired(cmd=args, timeout=self._timeout) from e profile.sample() ptimeout = 0.01 exitcode = p.returncode @@ -1154,7 +1154,7 @@ class CurlClient: pass elif insecure: args.append('--insecure') - elif active_options and ("--cacert" in active_options or \ + elif active_options and ("--cacert" in active_options or "--capath" in active_options): pass elif u.hostname: diff --git a/tests/http/testenv/dante.py b/tests/http/testenv/dante.py index 9e1d63ce00..83323bea11 100644 --- a/tests/http/testenv/dante.py +++ b/tests/http/testenv/dante.py @@ -145,11 +145,11 @@ class Dante: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): conf = [ diff --git a/tests/http/testenv/env.py b/tests/http/testenv/env.py index a92bc23ef3..d7eaac7bec 100644 --- a/tests/http/testenv/env.py +++ b/tests/http/testenv/env.py @@ -528,18 +528,15 @@ class Env: if Env.have_h2_curl(): if Env.have_h3(): return ['http/1.1', 'h2', 'h3'] - else: - return ['http/1.1', 'h2'] - else: - return ['http/1.1'] + return ['http/1.1', 'h2'] + return ['http/1.1'] @staticmethod def http_h1_h2_protos() -> List[str]: # http 1+2 protocols we can test if Env.have_h2_curl(): return ['http/1.1', 'h2'] - else: - return ['http/1.1'] + return ['http/1.1'] @staticmethod def http_mplx_protos() -> List[str]: @@ -547,10 +544,8 @@ class Env: if Env.have_h2_curl(): if Env.have_h3(): return ['h2', 'h3'] - else: - return ['h2'] - else: - return [] + return ['h2'] + return [] @staticmethod def have_h3() -> bool: diff --git a/tests/http/testenv/httpd.py b/tests/http/testenv/httpd.py index 52578be7d6..7a353791ad 100644 --- a/tests/http/testenv/httpd.py +++ b/tests/http/testenv/httpd.py @@ -250,11 +250,11 @@ class Httpd: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): domain1 = self.env.domain1 @@ -503,12 +503,11 @@ class Httpd: ' Require user proxy', ' ', ] - else: - return [ - ' ', - ' Require ip 127.0.0.1', - ' ', - ] + return [ + ' ', + ' Require ip 127.0.0.1', + ' ', + ] def _get_log_level(self): if self.env.verbose > 3: diff --git a/tests/http/testenv/nghttpx.py b/tests/http/testenv/nghttpx.py index 950d567f24..0393c0d078 100644 --- a/tests/http/testenv/nghttpx.py +++ b/tests/http/testenv/nghttpx.py @@ -193,11 +193,11 @@ class Nghttpx: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): with open(self._conf_file, 'w') as fd: diff --git a/tests/http/testenv/sshd.py b/tests/http/testenv/sshd.py index 18e5e2cfd7..10384fd5e8 100644 --- a/tests/http/testenv/sshd.py +++ b/tests/http/testenv/sshd.py @@ -258,11 +258,11 @@ class Sshd: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): conf = [ diff --git a/tests/http/testenv/vsftpd.py b/tests/http/testenv/vsftpd.py index acf86b989e..09cfc0826a 100644 --- a/tests/http/testenv/vsftpd.py +++ b/tests/http/testenv/vsftpd.py @@ -174,11 +174,11 @@ class VsFTPD: def _rmf(self, path): if os.path.exists(path): - return os.remove(path) + os.remove(path) def _mkpath(self, path): if not os.path.exists(path): - return os.makedirs(path) + os.makedirs(path) def _write_config(self): self._mkpath(self._docs_dir) diff --git a/tests/negtelnetserver.py b/tests/negtelnetserver.py index 90ea58df5a..f85899bc31 100755 --- a/tests/negtelnetserver.py +++ b/tests/negtelnetserver.py @@ -110,7 +110,7 @@ class NegotiatingTelnetHandler(socketserver.BaseRequestHandler): except IOError: log.exception("IOError hit during request") -class Negotiator(object): +class Negotiator: NO_NEG = 0 START_NEG = 1 WILL = 2 @@ -238,7 +238,7 @@ class Negotiator(object): log.debug("Sending WONT %s", option_str) self.send_iac([NegTokens.WONT, NegOptions.to_val(option_str)]) -class NegBase(object): +class NegBase: @classmethod def to_val(cls, name): return getattr(cls, name) @@ -330,7 +330,7 @@ def setup_logging(options): stdout_handler.setLevel(logging.DEBUG) root_logger.addHandler(stdout_handler) -class ScriptRC(object): +class ScriptRC: """Enum for script return codes.""" SUCCESS = 0 diff --git a/tests/smbserver.py b/tests/smbserver.py index 2e0ada06d2..14d9e2e352 100755 --- a/tests/smbserver.py +++ b/tests/smbserver.py @@ -349,9 +349,9 @@ class TestSmbServer(imp_smbserver.SMBSERVER): self.write_to_fid(fid, contents) return fid, filename - except Exception: + except Exception as e: log.exception("Failed to make test file") - raise SmbError(STATUS_NO_SUCH_FILE, "Failed to make test file") + raise SmbError(STATUS_NO_SUCH_FILE, "Failed to make test file") from e class SmbError(Exception): @@ -360,7 +360,7 @@ class SmbError(Exception): self.error_code = error_code -class ScriptRC(object): +class ScriptRC: """Enum for script return codes.""" SUCCESS = 0 diff --git a/tests/util.py b/tests/util.py index d2826d18d5..942a590a8f 100755 --- a/tests/util.py +++ b/tests/util.py @@ -59,7 +59,7 @@ class ClosingFileHandler(logging.StreamHandler): self.release() return result -class TestData(object): +class TestData: def __init__(self, data_folder): self.data_folder = data_folder