From 006517ea2d69d3f0d9e3de2eb67bfb4d32f20551 Mon Sep 17 00:00:00 2001 From: Tomas Fratrik Date: Wed, 13 Aug 2025 10:46:10 +0200 Subject: [PATCH 32/55] pylint: enable consider-using-with Emitted when a resource-allocating assignment or call could be replaced by a 'with' block. Enabling this warning enforces using 'with' to ensure resources are properly released even if an exception occurs. * ifcfgscanner: use StringIO in tests instead of mock_open for iteration support with 'with open' Jira: RHELMISC-16038 --- .pylintrc | 1 - .../ifcfgscanner/libraries/ifcfgscanner.py | 27 +++++++++---------- .../tests/unit_test_ifcfgscanner.py | 8 +++--- .../luksscanner/tests/test_luksdump_parser.py | 8 +++--- .../scansaphana/tests/test_scansaphana.py | 6 ++--- .../system_upgrade/common/libraries/guards.py | 2 +- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/.pylintrc b/.pylintrc index e54d9a54..fd770061 100644 --- a/.pylintrc +++ b/.pylintrc @@ -51,7 +51,6 @@ disable= super-with-arguments, # required in python 2 raise-missing-from, # no 'raise from' in python 2 use-a-generator, # cannot be modified because of Python2 support - consider-using-with, # on bunch spaces we cannot change that... duplicate-string-formatting-argument, # TMP: will be fixed in close future consider-using-f-string, # sorry, not gonna happen, still have to support py2 use-dict-literal, diff --git a/repos/system_upgrade/common/actors/ifcfgscanner/libraries/ifcfgscanner.py b/repos/system_upgrade/common/actors/ifcfgscanner/libraries/ifcfgscanner.py index 683327b3..f0c8b847 100644 --- a/repos/system_upgrade/common/actors/ifcfgscanner/libraries/ifcfgscanner.py +++ b/repos/system_upgrade/common/actors/ifcfgscanner/libraries/ifcfgscanner.py @@ -18,23 +18,22 @@ def process_ifcfg(filename, secrets=False): return None properties = [] - for line in open(filename).readlines(): - try: - (name, value) = line.split("#")[0].strip().split("=") + with open(filename) as f: + for line in f: + try: + (name, value) = line.split("#")[0].strip().split("=") + except ValueError: + # We're not interested in lines that are not + # simple assignments. Play it safe. + continue + if secrets: value = None - except ValueError: - # We're not interested in lines that are not - # simple assignments. Play it safe. - continue - - # Deal with simple quoting. We don't expand anything, nor do - # multiline strings or anything of that sort. - if value is not None and len(value) > 1 and value[0] == value[-1]: - if value.startswith('"') or value.startswith("'"): + elif len(value) > 1 and value[0] in ('"', "'") and value[0] == value[-1]: + # Deal with simple quoting. We don't expand anything, nor do + # multiline strings or anything of that sort. value = value[1:-1] - - properties.append(IfCfgProperty(name=name, value=value)) + properties.append(IfCfgProperty(name=name, value=value)) return properties diff --git a/repos/system_upgrade/common/actors/ifcfgscanner/tests/unit_test_ifcfgscanner.py b/repos/system_upgrade/common/actors/ifcfgscanner/tests/unit_test_ifcfgscanner.py index d3b4846f..d996de84 100644 --- a/repos/system_upgrade/common/actors/ifcfgscanner/tests/unit_test_ifcfgscanner.py +++ b/repos/system_upgrade/common/actors/ifcfgscanner/tests/unit_test_ifcfgscanner.py @@ -1,5 +1,6 @@ import errno import textwrap +from io import StringIO from os.path import basename import mock @@ -63,8 +64,7 @@ def test_ifcfg1(monkeypatch): KEY_TYPE=key """) - mock_config = mock.mock_open(read_data=ifcfg_file) - with mock.patch(_builtins_open, mock_config): + with mock.patch(_builtins_open, return_value=StringIO(ifcfg_file)): monkeypatch.setattr(ifcfgscanner, "listdir", _listdir_ifcfg) monkeypatch.setattr(ifcfgscanner.path, "exists", _exists_ifcfg) monkeypatch.setattr(api, "produce", produce_mocked()) @@ -110,8 +110,8 @@ def test_ifcfg_key(monkeypatch): Report ifcfg secrets from keys- file. """ - mock_config = mock.mock_open(read_data="KEY_PASSPHRASE1=Hell0") - with mock.patch(_builtins_open, mock_config): + file_data = "KEY_PASSPHRASE1=Hell0" + with mock.patch(_builtins_open, side_effect=lambda *a, **k: StringIO(file_data)): monkeypatch.setattr(ifcfgscanner, "listdir", _listdir_ifcfg) monkeypatch.setattr(ifcfgscanner.path, "exists", _exists_keys) monkeypatch.setattr(api, "produce", produce_mocked()) diff --git a/repos/system_upgrade/common/actors/luksscanner/tests/test_luksdump_parser.py b/repos/system_upgrade/common/actors/luksscanner/tests/test_luksdump_parser.py index 4b190149..f0482eef 100644 --- a/repos/system_upgrade/common/actors/luksscanner/tests/test_luksdump_parser.py +++ b/repos/system_upgrade/common/actors/luksscanner/tests/test_luksdump_parser.py @@ -7,8 +7,8 @@ CUR_DIR = os.path.dirname(os.path.abspath(__file__)) def test_luksdump_parser_luks1(current_actor_context): - f = open(os.path.join(CUR_DIR, 'files/luksDump_nvme0n1p3_luks1.txt')) - parsed_dict = LuksDumpParser.parse(f.readlines()) + with open(os.path.join(CUR_DIR, 'files/luksDump_nvme0n1p3_luks1.txt')) as f: + parsed_dict = LuksDumpParser.parse(f.readlines()) assert parsed_dict["Version"] == "1" assert parsed_dict["Cipher name"] == "aes" @@ -39,8 +39,8 @@ def test_luksdump_parser_luks1(current_actor_context): def test_luksdump_parser_luks2_tokens(current_actor_context): - f = open(os.path.join(CUR_DIR, 'files/luksDump_nvme0n1p3_luks2_tokens.txt')) - parsed_dict = LuksDumpParser.parse(f.readlines()) + with open(os.path.join(CUR_DIR, 'files/luksDump_nvme0n1p3_luks2_tokens.txt')) as f: + parsed_dict = LuksDumpParser.parse(f.readlines()) assert parsed_dict["Version"] == "2" assert parsed_dict["Epoch"] == "9" diff --git a/repos/system_upgrade/common/actors/scansaphana/tests/test_scansaphana.py b/repos/system_upgrade/common/actors/scansaphana/tests/test_scansaphana.py index 0b55c9fb..38a1cae7 100644 --- a/repos/system_upgrade/common/actors/scansaphana/tests/test_scansaphana.py +++ b/repos/system_upgrade/common/actors/scansaphana/tests/test_scansaphana.py @@ -77,9 +77,9 @@ class SubprocessCall(object): assert args[0][0:3] == ['sudo', '-u', self.admusername] cmd = args[0][3:] kwargs.pop('checked', None) - p = subprocess.Popen(cmd, stdout=subprocess.PIPE) - p.wait() - return {'exit_code': p.returncode, 'stdout': p.stdout.read()} + with subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as p: + stdout, stderr = p.communicate() + return {'exit_code': p.returncode, 'stdout': stdout.decode('utf-8'), 'stderr': stderr.decode('utf-8')} def test_scansaphana_get_instance_status(monkeypatch): diff --git a/repos/system_upgrade/common/libraries/guards.py b/repos/system_upgrade/common/libraries/guards.py index c8001817..ea2bf4dd 100644 --- a/repos/system_upgrade/common/libraries/guards.py +++ b/repos/system_upgrade/common/libraries/guards.py @@ -34,7 +34,7 @@ def guarded_execution(*guards): def connection_guard(url='https://example.com'): def closure(): try: - urlopen(url) + urlopen(url) # pylint: disable=consider-using-with return None except URLError as e: cause = '''Failed to open url '{url}' with error: {error}'''.format(url=url, error=e) -- 2.51.1