On Fri, Jan 15, 2021 at 2:43 PM Max Reitz <mre...@redhat.com> wrote: > > Instead of checking iotests.py only, check all Python files in the > qemu-iotests/ directory. Of course, most of them do not pass, so there > is an extensive skip list for now. (The only files that do pass are > 209, 254, 283, and iotests.py.) > > (Alternatively, we could have the opposite, i.e. an explicit list of > files that we do want to check, but I think it is better to check files > by default.) > > Unless started in debug mode (./check -d), the output has no information > on which files are tested, so we will not have a problem e.g. with > backports, where some files may be missing when compared to upstream. > > Besides the technical rewrite, some more things are changed: > > - For the pylint invocation, PYTHONPATH is adjusted. This mirrors > setting MYPYPATH for mypy. > > - Also, MYPYPATH is now derived from PYTHONPATH, so that we include > paths set by the environment. Maybe at some point we want to let the > check script add '../../python/' to PYTHONPATH so that iotests.py does > not need to do that. > > - Passing --notes=FIXME,XXX to pylint suppresses warnings for TODO > comments. TODO is fine, we do not need 297 to complain about such > comments. > > - The "Success" line from mypy's output is suppressed, because (A) it > does not add useful information, and (B) it would leak information > about the files having been tested to the reference output, which we > decidedly do not want. > > Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Signed-off-by: Max Reitz <mre...@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > tests/qemu-iotests/297 | 110 +++++++++++++++++++++++++++++-------- > tests/qemu-iotests/297.out | 5 +- > 2 files changed, 90 insertions(+), 25 deletions(-) > > diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 > index 5c5420712b..fa9e2cac78 100755 > --- a/tests/qemu-iotests/297 > +++ b/tests/qemu-iotests/297 > @@ -1,4 +1,4 @@ > -#!/usr/bin/env bash > +#!/usr/bin/env python3 > # > # Copyright (C) 2020 Red Hat, Inc. > # > @@ -15,30 +15,96 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > -seq=$(basename $0) > -echo "QA output created by $seq" > +import os > +import re > +import shutil > +import subprocess > +import sys > > -status=1 # failure is the default! > +import iotests > > -# get standard environment > -. ./common.rc > > -if ! type -p "pylint-3" > /dev/null; then > - _notrun "pylint-3 not found" > -fi > -if ! type -p "mypy" > /dev/null; then > - _notrun "mypy not found" > -fi > +# TODO: Empty this list! > +SKIP_FILES = ( > + '030', '040', '041', '044', '045', '055', '056', '057', '065', '093', > + '096', '118', '124', '129', '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', '300', '302', '303', '304', '307', > + 'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py' > +) > > -pylint-3 --score=n iotests.py > > -MYPYPATH=../../python/ 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 iotests.py > +def is_python_file(filename): > + if not os.path.isfile(filename): > + return False > > -# success, all done > -echo "*** done" > -rm -f $seq.full > -status=0 > + 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)] > + > + 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() > + try: > + env['PYTHONPATH'] += ':../../python/'
Do you have any objection to using os.path.dirname and os.path.join here? This would make the code more pythonic. > + except KeyError: > + env['PYTHONPATH'] = '../../python/' Same here. You could do it once, before the 'try' and use it inside. Other than that, Reviewed-by: Willian Rampazzo <willi...@redhat.com>