| Message ID | 20210114170304.87507-3-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Fix 129 and expand 297’s reach | expand |
14.01.2021 20:02, Max Reitz 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 <vsementsov@virtuozzo.com> > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/297 | 109 +++++++++++++++++++++++++++++-------- > tests/qemu-iotests/297.out | 5 +- > 2 files changed, 89 insertions(+), 25 deletions(-) > > diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 > index 5c5420712b..bfa26d280b 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,95 @@ > # 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 PEP8 asks for two spaces before inline comment > + return False > + and two empty lines here (good ALE vim plugin runs flake8, mypy and pylint asynchronously for me and shows these things) > +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))) O, good. > + > + print('=== pylint ===') > + sys.stdout.flush() Should not hurt.. But why? Should be flushed anyway as print will put a '\n'.. And why you care about it at all? > + > + # Todo notes are fine, but fixme's or xxx's should probably just be > + # fixed (in tests, at least) > + env = os.environ > + try: > + env['PYTHONPATH'] += ':../../python/' > + except KeyError: > + env['PYTHONPATH'] = '../../python/' > + subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files), > + env=env, check=False) as I understand, you don't need env argument, as os.environ is inherited by default. > + > + 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', > + filename), > + env=env, > + check=False, > + stdout=subprocess.PIPE, > + stderr=subprocess.STDOUT, > + text=True) Can you really use "text" ? We require python 3.6 (in check), but text comes from 3.7.. It may break some test environments. I think we need old good universal_newlines=True which is the same. With s/text/universal_newlines/: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > + > + 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(run_linters) > diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out > index 6acc843649..f2e1314d10 100644 > --- a/tests/qemu-iotests/297.out > +++ b/tests/qemu-iotests/297.out > @@ -1,3 +1,2 @@ > -QA output created by 297 > -Success: no issues found in 1 source file > -*** done > +=== pylint === > +=== mypy === >
On 15.01.21 12:13, Vladimir Sementsov-Ogievskiy wrote: > 14.01.2021 20:02, Max Reitz 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 <vsementsov@virtuozzo.com> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/297 | 109 +++++++++++++++++++++++++++++-------- >> tests/qemu-iotests/297.out | 5 +- >> 2 files changed, 89 insertions(+), 25 deletions(-) >> >> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 >> index 5c5420712b..bfa26d280b 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,95 @@ >> # 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 > > PEP8 asks for two spaces before inline comment Wow. I really hate PEP8. >> + return False >> + > > and two empty lines here > > (good ALE vim plugin runs flake8, mypy and pylint asynchronously for me > and shows these things) I’d like to argue that that isn’t good, but I need to quench my anger. Perhaps one day I can quench it sufficiently to install ALE myself. >> +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))) > > O, good. > >> + >> + print('=== pylint ===') >> + sys.stdout.flush() > > Should not hurt.. But why? Should be flushed anyway as print will put a > '\n'.. And why you care about it at all? When pylint fails, I can see its result above the '=== pylint ===' line, even though its output is on stdout. I suspect the Python output indeed isn’t flushed on \n. (Test it by dropping the flush(), and then also e.g. 041 from the ignore list. Perhaps you get a different result, but for me, the errors appear above the '=== pylint ===' line.) >> + >> + # Todo notes are fine, but fixme's or xxx's should probably just be >> + # fixed (in tests, at least) >> + env = os.environ >> + try: >> + env['PYTHONPATH'] += ':../../python/' >> + except KeyError: >> + env['PYTHONPATH'] = '../../python/' >> + subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', >> *files), >> + env=env, check=False) > > as I understand, you don't need env argument, as os.environ is inherited > by default. Yes, but os.environ['PYTHONPATH'] doesn’t include ../../python/, which is why I copy it. On second though, I don’t copy it. I think the “env = os.environ” line should be “env = os.environ.copy()”, actually, or I’ll modify 297’s own PYTHONPATH. >> + >> + 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', >> + filename), >> + env=env, >> + check=False, >> + stdout=subprocess.PIPE, >> + stderr=subprocess.STDOUT, >> + text=True) > > Can you really use "text" ? We require python 3.6 (in check), but text > comes from 3.7.. Oh well. My fault for just reading the current docs. > It may break some test environments. I think we need old good > universal_newlines=True which is the same. Good enough for me. > With s/text/universal_newlines/: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Thanks! I’ll clean up the PEP8 violations, make the “env = os.environ” line an “env = os.environ.copy()”, and use universal_newlines here. Max
15.01.2021 14:57, Max Reitz wrote: > On 15.01.21 12:13, Vladimir Sementsov-Ogievskiy wrote: >> 14.01.2021 20:02, Max Reitz 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 <vsementsov@virtuozzo.com> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> --- >>> tests/qemu-iotests/297 | 109 +++++++++++++++++++++++++++++-------- >>> tests/qemu-iotests/297.out | 5 +- >>> 2 files changed, 89 insertions(+), 25 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 >>> index 5c5420712b..bfa26d280b 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,95 @@ >>> # 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 >> >> PEP8 asks for two spaces before inline comment > > Wow. I really hate PEP8. Wow, it's unexpected :) I like it since first meet.. > >>> + return False >>> + >> >> and two empty lines here >> >> (good ALE vim plugin runs flake8, mypy and pylint asynchronously for me and shows these things) > > I’d like to argue that that isn’t good, but I need to quench my anger. Perhaps one day I can quench it sufficiently to install ALE myself. > >>> +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))) >> >> O, good. >> >>> + >>> + print('=== pylint ===') >>> + sys.stdout.flush() >> >> Should not hurt.. But why? Should be flushed anyway as print will put a '\n'.. And why you care about it at all? > > When pylint fails, I can see its result above the '=== pylint ===' line, even though its output is on stdout. I suspect the Python output indeed isn’t flushed on \n. > > (Test it by dropping the flush(), and then also e.g. 041 from the ignore list. Perhaps you get a different result, but for me, the errors appear above the '=== pylint ===' line.) Checked. Hmm, you are right. Interesting. Probably it explains similar issues with failed iotests output I see sometimes.. > >>> + >>> + # Todo notes are fine, but fixme's or xxx's should probably just be >>> + # fixed (in tests, at least) >>> + env = os.environ >>> + try: >>> + env['PYTHONPATH'] += ':../../python/' >>> + except KeyError: >>> + env['PYTHONPATH'] = '../../python/' >>> + subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files), >>> + env=env, check=False) >> >> as I understand, you don't need env argument, as os.environ is inherited by default. > > Yes, but os.environ['PYTHONPATH'] doesn’t include ../../python/, which is why I copy it. > > On second though, I don’t copy it. I think the “env = os.environ” line should be “env = os.environ.copy()”, actually, or I’ll modify 297’s own PYTHONPATH. I think modifying os.environ won't hurt in this case.. But copying is more strict of course. > >>> + >>> + 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', >>> + filename), >>> + env=env, >>> + check=False, >>> + stdout=subprocess.PIPE, >>> + stderr=subprocess.STDOUT, >>> + text=True) >> >> Can you really use "text" ? We require python 3.6 (in check), but text comes from 3.7.. > > Oh well. My fault for just reading the current docs. > >> It may break some test environments. I think we need old good universal_newlines=True which is the same. > > Good enough for me. > >> With s/text/universal_newlines/: >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > Thanks! I’ll clean up the PEP8 violations, make the “env = os.environ” line an “env = os.environ.copy()”, and use universal_newlines here. > OK, thanks, keep my r-b with it.
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297 index 5c5420712b..bfa26d280b 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,95 @@ # 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 + try: + env['PYTHONPATH'] += ':../../python/' + except KeyError: + env['PYTHONPATH'] = '../../python/' + 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', + filename), + env=env, + check=False, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=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(run_linters) diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out index 6acc843649..f2e1314d10 100644 --- a/tests/qemu-iotests/297.out +++ b/tests/qemu-iotests/297.out @@ -1,3 +1,2 @@ -QA output created by 297 -Success: no issues found in 1 source file -*** done +=== pylint === +=== mypy ===
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 <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/297 | 109 +++++++++++++++++++++++++++++-------- tests/qemu-iotests/297.out | 5 +- 2 files changed, 89 insertions(+), 25 deletions(-)