Refactor the core function of the linting configuration out of 297 and into a new file called linters.py.
Now, linters.py represents an invocation of the linting scripts that more resembles a "normal" execution of pylint/mypy, like you'd expect to use if 'qemu' was a bona-fide package you obtained from PyPI. 297, by contrast, now represents the iotests-specific configuration bits you need to get it to function correctly as a part of iotests, and with 'qemu' as a namespace package that isn't "installed" to the current environment, but just lives elsewhere in our source tree. By doing this, we will able to run the same linting configuration from the Python CI tests without calling iotest logging functions or messing around with PYTHONPATH / MYPYPATH. iotest 297 continues to operate in a standalone fashion for now -- presumably, it's convenient for block maintainers and contributors to run in this manner. See the following commit for how this is used from the Python packaging side. Signed-off-by: John Snow <js...@redhat.com> --- - It's a big glob of a patch. Sorry. I can work it into smaller pieces if the idea is well received. - I change the invocations of mypy/pylint to "python3 -m pylint" and "python3 -m mypy" respectively, which causes these linters to use the virtual environment's preferred version. This forces the test to use the test environments curated by the CI jobs. - If you have installed Fedora's pylint package that provides "pylint-3", the above trick will still work correctly. - Checking for "pylint-3" specifically in 297 was left alone. Theoretically, this check could be broadened to simply look for the presence of a 'pylint' module to allow it to be more permissive. Signed-off-by: John Snow <js...@redhat.com> --- tests/qemu-iotests/297 | 88 ++++------------------- tests/qemu-iotests/linters.py | 130 ++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 75 deletions(-) create mode 100644 tests/qemu-iotests/linters.py diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 index 433b732336..5c753279fc 100755 --- a/tests/qemu-iotests/297 +++ b/tests/qemu-iotests/297 @@ -17,98 +17,36 @@ # along with this program. If not, see <http://www.gnu.org/licenses/>. import os -import re import shutil -import subprocess -import sys import iotests +import linters -# TODO: Empty this list! -SKIP_FILES = ( - '030', '040', '041', '044', '045', '055', '056', '057', '065', '093', - '096', '118', '124', '132', '136', '139', '147', '148', '149', - '151', '152', '155', '163', '165', '169', '194', '196', '199', '202', - '203', '205', '206', '207', '208', '210', '211', '212', '213', '216', - '218', '219', '222', '224', '228', '234', '235', '236', '237', '238', - '240', '242', '245', '246', '248', '255', '256', '257', '258', '260', - '262', '264', '266', '274', '277', '280', '281', '295', '296', '298', - '299', '302', '303', '304', '307', - 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py' -) - - -def is_python_file(filename): - if not os.path.isfile(filename): - return False - - if filename.endswith('.py'): - return True - - with open(filename) as f: - try: - first_line = f.readline() - return re.match('^#!.*python', first_line) is not None - except UnicodeDecodeError: # Ignore binary files - return False - - -def run_linters(): - files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES)) - if is_python_file(filename)] +def main(): + files = linters.get_test_files() iotests.logger.debug('Files to be checked:') iotests.logger.debug(', '.join(sorted(files))) - print('=== pylint ===') - sys.stdout.flush() - - # Todo notes are fine, but fixme's or xxx's should probably just be - # fixed (in tests, at least) env = os.environ.copy() - qemu_module_path = os.path.join(os.path.dirname(__file__), - '..', '..', 'python') + qemu_module_path = os.path.join( + os.path.dirname(__file__), + '..', '..', 'python' + ) + try: env['PYTHONPATH'] += os.pathsep + qemu_module_path except KeyError: env['PYTHONPATH'] = qemu_module_path - subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files), - env=env, check=False) - print('=== mypy ===') - sys.stdout.flush() - - # We have to call mypy separately for each file. Otherwise, it - # will interpret all given files as belonging together (i.e., they - # may not both define the same classes, etc.; most notably, they - # must not both define the __main__ module). env['MYPYPATH'] = env['PYTHONPATH'] - for filename in files: - p = subprocess.run(('mypy', - '--warn-unused-configs', - '--disallow-subclassing-any', - '--disallow-any-generics', - '--disallow-incomplete-defs', - '--disallow-untyped-decorators', - '--no-implicit-optional', - '--warn-redundant-casts', - '--warn-unused-ignores', - '--no-implicit-reexport', - '--namespace-packages', - filename), - env=env, - check=False, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True) - if p.returncode != 0: - print(p.stdout) + for linter in ('pylint-3', 'mypy'): + if shutil.which(linter) is None: + iotests.notrun(f'{linter} not found') + iotests.script_main(lambda: linters.run_linters(files, env=env)) -for linter in ('pylint-3', 'mypy'): - if shutil.which(linter) is None: - iotests.notrun(f'{linter} not found') -iotests.script_main(run_linters) +main() diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py new file mode 100644 index 0000000000..1bbcfd1088 --- /dev/null +++ b/tests/qemu-iotests/linters.py @@ -0,0 +1,130 @@ +# Copyright (C) 2020 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +import os +import re +import subprocess +import sys +from typing import List, Mapping, Optional + + +# TODO: Empty this list! +SKIP_FILES = ( + '030', '040', '041', '044', '045', '055', '056', '057', '065', '093', + '096', '118', '124', '132', '136', '139', '147', '148', '149', + '151', '152', '155', '163', '165', '169', '194', '196', '199', '202', + '203', '205', '206', '207', '208', '210', '211', '212', '213', '216', + '218', '219', '222', '224', '228', '234', '235', '236', '237', '238', + '240', '242', '245', '246', '248', '255', '256', '257', '258', '260', + '262', '264', '266', '274', '277', '280', '281', '295', '296', '298', + '299', '302', '303', '304', '307', + 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py' +) + + +def is_python_file(filename: str, directory: str = '.') -> bool: + filepath = os.path.join(directory, filename) + + if not os.path.isfile(filepath): + return False + + if filename.endswith('.py'): + return True + + with open(filepath) as f: + try: + first_line = f.readline() + return re.match('^#!.*python', first_line) is not None + except UnicodeDecodeError: # Ignore binary files + return False + + +def get_test_files(directory: str = '.') -> List[str]: + return [ + f for f in (set(os.listdir(directory)) - set(SKIP_FILES)) + if is_python_file(f, directory) + ] + + +def run_linters( + files: List[str], + directory: str = '.', + env: Optional[Mapping[str, str]] = None, +) -> int: + ret = 0 + + print('=== pylint ===') + sys.stdout.flush() + + # Todo notes are fine, but fixme's or xxx's should probably just be + # fixed (in tests, at least) + p = subprocess.run( + ('python3', '-m', 'pylint', '--score=n', '--notes=FIXME,XXX', *files), + cwd=directory, + env=env, + check=False, + universal_newlines=True, + ) + ret += p.returncode + + print('=== mypy ===') + sys.stdout.flush() + + # We have to call mypy separately for each file. Otherwise, it + # will interpret all given files as belonging together (i.e., they + # may not both define the same classes, etc.; most notably, they + # must not both define the __main__ module). + for filename in files: + p = subprocess.run( + ( + 'python3', '-m', 'mypy', + '--warn-unused-configs', + '--disallow-subclassing-any', + '--disallow-any-generics', + '--disallow-incomplete-defs', + '--disallow-untyped-decorators', + '--no-implicit-optional', + '--warn-redundant-casts', + '--warn-unused-ignores', + '--no-implicit-reexport', + '--namespace-packages', + filename, + ), + cwd=directory, + env=env, + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True + ) + + ret += p.returncode + if p.returncode != 0: + print(p.stdout) + + return ret + + +def main() -> int: + """ + Used by the Python CI system as an entry point to run these linters. + """ + directory = os.path.dirname(os.path.realpath(__file__)) + files = get_test_files(directory) + return run_linters(files, directory) + + +if __name__ == '__main__': + sys.exit(main()) -- 2.31.1
