* [PATCH v2 01/17] iotests: use with-statement for open() calls
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:55 ` Philippe Mathieu-Daudé
2021-07-20 17:33 ` [PATCH v2 02/17] iotests: use subprocess.DEVNULL instead of open("/dev/null") John Snow
` (17 subsequent siblings)
18 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Silences a new pylint warning. The dangers of *not* doing this are
somewhat unclear; I believe the file object gets garbage collected
eventually, but possibly the way in which it happens is
non-deterministic. Maybe this is a valid warning, but if there are
consequences of not doing it, I am not aware of them at present.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/iotests.py | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 89663dac06d..beadf5c821c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1075,7 +1075,8 @@ def notrun(reason):
# Each test in qemu-iotests has a number ("seq")
seq = os.path.basename(sys.argv[0])
- open('%s/%s.notrun' % (output_dir, seq), 'w').write(reason + '\n')
+ with open('%s/%s.notrun' % (output_dir, seq), 'w') as outfile:
+ outfile.write(reason + '\n')
logger.warning("%s not run: %s", seq, reason)
sys.exit(0)
@@ -1088,8 +1089,8 @@ def case_notrun(reason):
# Each test in qemu-iotests has a number ("seq")
seq = os.path.basename(sys.argv[0])
- open('%s/%s.casenotrun' % (output_dir, seq), 'a').write(
- ' [case not run] ' + reason + '\n')
+ with open('%s/%s.casenotrun' % (output_dir, seq), 'a') as outfile:
+ outfile.write(' [case not run] ' + reason + '\n')
def _verify_image_format(supported_fmts: Sequence[str] = (),
unsupported_fmts: Sequence[str] = ()) -> None:
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 01/17] iotests: use with-statement for open() calls
2021-07-20 17:33 ` [PATCH v2 01/17] iotests: use with-statement for open() calls John Snow
@ 2021-07-20 17:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-20 17:55 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
On 7/20/21 7:33 PM, John Snow wrote:
> Silences a new pylint warning. The dangers of *not* doing this are
> somewhat unclear; I believe the file object gets garbage collected
> eventually, but possibly the way in which it happens is
> non-deterministic. Maybe this is a valid warning, but if there are
> consequences of not doing it, I am not aware of them at present.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 02/17] iotests: use subprocess.DEVNULL instead of open("/dev/null")
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
2021-07-20 17:33 ` [PATCH v2 01/17] iotests: use with-statement for open() calls John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:57 ` Philippe Mathieu-Daudé
2021-07-20 17:33 ` [PATCH v2 03/17] iotests/mirror-top-perms: Adjust import paths John Snow
` (16 subsequent siblings)
18 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Avoids a warning from pylint not to use open() outside of a
with-statement, and is ... probably more portable anyway. Not that I
think we care too much about running tests *on* Windows, but... eh.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/iotests.py | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index beadf5c821c..64f9b688f55 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -219,18 +219,18 @@ def qemu_io_silent(*args):
default_args = qemu_io_args
args = default_args + list(args)
- exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'))
- if exitcode < 0:
+ result = subprocess.run(args, stdout=subprocess.DEVNULL, check=False)
+ if result.returncode < 0:
sys.stderr.write('qemu-io received signal %i: %s\n' %
- (-exitcode, ' '.join(args)))
- return exitcode
+ (-result.returncode, ' '.join(args)))
+ return result.returncode
def qemu_io_silent_check(*args):
'''Run qemu-io and return the true if subprocess returned 0'''
args = qemu_io_args + list(args)
- exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'),
- stderr=subprocess.STDOUT)
- return exitcode == 0
+ result = subprocess.run(args, stdout=subprocess.DEVNULL,
+ stderr=subprocess.STDOUT, check=False)
+ return result.returncode == 0
class QemuIoInteractive:
def __init__(self, *args):
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 02/17] iotests: use subprocess.DEVNULL instead of open("/dev/null")
2021-07-20 17:33 ` [PATCH v2 02/17] iotests: use subprocess.DEVNULL instead of open("/dev/null") John Snow
@ 2021-07-20 17:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-20 17:57 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
On 7/20/21 7:33 PM, John Snow wrote:
> Avoids a warning from pylint not to use open() outside of a
> with-statement, and is ... probably more portable anyway. Not that I
> think we care too much about running tests *on* Windows, but... eh.
But we should IMO, because the community expressed some interest
(see the increasing Windows fixes over last year and CI added).
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 03/17] iotests/mirror-top-perms: Adjust import paths
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
2021-07-20 17:33 ` [PATCH v2 01/17] iotests: use with-statement for open() calls John Snow
2021-07-20 17:33 ` [PATCH v2 02/17] iotests: use subprocess.DEVNULL instead of open("/dev/null") John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:57 ` Philippe Mathieu-Daudé
2021-07-20 17:33 ` [PATCH v2 04/17] iotests/migrate-bitmaps-postcopy-test: declare instance variables John Snow
` (15 subsequent siblings)
18 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Technically AbnormalShutdown isn't exported via
'qemu.machine.AbnormalShutdown', but instead from
'qemu.machine.machine.AbnormalShutdown'. Odd, I know, and it's on the
list to fix. For now, change the imports here.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/tests/mirror-top-perms | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index 451a0666f81..de181825901 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -25,7 +25,8 @@ from iotests import qemu_img
# Import qemu after iotests.py has amended sys.path
# pylint: disable=wrong-import-order
-import qemu
+from qemu import qmp
+from qemu.machine import machine
image_size = 1 * 1024 * 1024
@@ -47,7 +48,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
def tearDown(self):
try:
self.vm.shutdown()
- except qemu.machine.AbnormalShutdown:
+ except machine.AbnormalShutdown:
pass
if self.vm_b is not None:
@@ -102,7 +103,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
self.vm_b.launch()
print('ERROR: VM B launched successfully, this should not have '
'happened')
- except qemu.qmp.QMPConnectError:
+ except qmp.QMPConnectError:
assert 'Is another process using the image' in self.vm_b.get_log()
result = self.vm.qmp('block-job-cancel',
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 03/17] iotests/mirror-top-perms: Adjust import paths
2021-07-20 17:33 ` [PATCH v2 03/17] iotests/mirror-top-perms: Adjust import paths John Snow
@ 2021-07-20 17:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-20 17:57 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
On 7/20/21 7:33 PM, John Snow wrote:
> Technically AbnormalShutdown isn't exported via
> 'qemu.machine.AbnormalShutdown', but instead from
> 'qemu.machine.machine.AbnormalShutdown'. Odd, I know, and it's on the
> list to fix. For now, change the imports here.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/tests/mirror-top-perms | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 04/17] iotests/migrate-bitmaps-postcopy-test: declare instance variables
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (2 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 03/17] iotests/mirror-top-perms: Adjust import paths John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 05/17] iotests/migrate-bitmaps-test: delint John Snow
` (14 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index 584062b4128..78dc19e3e3d 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -115,6 +115,9 @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
self.vm_a_events = []
self.vm_b_events = []
+ self.discards1_sha256: str
+ self.all_discards_sha256: str
+
def start_postcopy(self):
""" Run migration until RESUME event on target. Return this event. """
for i in range(nb_bitmaps):
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 05/17] iotests/migrate-bitmaps-test: delint
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (3 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 04/17] iotests/migrate-bitmaps-postcopy-test: declare instance variables John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 06/17] iotests/297: modify is_python_file to work from any CWD John Snow
` (13 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Mostly uninteresting stuff. Move the test injections under a function
named main() so that the variables used during that process aren't in
the global scope.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/tests/migrate-bitmaps-test | 70 +++++++++++--------
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test
index a5c7bc83e0e..fd36ee4a245 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -19,12 +19,12 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
-import os
-import iotests
-import time
import itertools
import operator
+import os
import re
+
+import iotests
from iotests import qemu_img, qemu_img_create, Timeout
@@ -62,9 +62,10 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
params['persistent'] = True
result = vm.qmp('block-dirty-bitmap-add', **params)
- self.assert_qmp(result, 'return', {});
+ self.assert_qmp(result, 'return', {})
- def get_bitmap_hash(self, vm):
+ @classmethod
+ def get_bitmap_hash(cls, vm):
result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
node='drive0', name='bitmap0')
return result['return']['sha256']
@@ -73,10 +74,10 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
node='drive0', name='bitmap0')
if sha256:
- self.assert_qmp(result, 'return/sha256', sha256);
+ self.assert_qmp(result, 'return/sha256', sha256)
else:
self.assert_qmp(result, 'error/desc',
- "Dirty bitmap 'bitmap0' not found");
+ "Dirty bitmap 'bitmap0' not found")
def do_test_migration_resume_source(self, persistent, migrate_bitmaps):
granularity = 512
@@ -106,7 +107,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
break
while True:
result = self.vm_a.qmp('query-status')
- if (result['return']['status'] == 'postmigrate'):
+ if result['return']['status'] == 'postmigrate':
break
# test that bitmap is still here
@@ -216,25 +217,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
def inject_test_case(klass, name, method, *args, **kwargs):
mc = operator.methodcaller(method, *args, **kwargs)
- setattr(klass, 'test_' + method + name, lambda self: mc(self))
-
-for cmb in list(itertools.product((True, False), repeat=5)):
- name = ('_' if cmb[0] else '_not_') + 'persistent_'
- name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
- name += '_online' if cmb[2] else '_offline'
- name += '_shared' if cmb[3] else '_nonshared'
- if (cmb[4]):
- name += '__pre_shutdown'
-
- inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
- *list(cmb))
-
-for cmb in list(itertools.product((True, False), repeat=2)):
- name = ('_' if cmb[0] else '_not_') + 'persistent_'
- name += ('_' if cmb[1] else '_not_') + 'migbitmap'
-
- inject_test_case(TestDirtyBitmapMigration, name,
- 'do_test_migration_resume_source', *list(cmb))
+ setattr(klass, 'test_' + method + name, mc)
class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
@@ -270,7 +253,8 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'return', {})
# Check that the bitmaps are there
- for node in self.vm.qmp('query-named-block-nodes', flat=True)['return']:
+ nodes = self.vm.qmp('query-named-block-nodes', flat=True)['return']
+ for node in nodes:
if 'node0' in node['node-name']:
self.assert_qmp(node, 'dirty-bitmaps[0]/name', 'bmap0')
@@ -287,7 +271,7 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
"""
Continue the source after migration.
"""
- result = self.vm.qmp('migrate', uri=f'exec: cat > /dev/null')
+ result = self.vm.qmp('migrate', uri='exec: cat > /dev/null')
self.assert_qmp(result, 'return', {})
with Timeout(10, 'Migration timeout'):
@@ -297,6 +281,30 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'return', {})
+def main():
+ for cmb in list(itertools.product((True, False), repeat=5)):
+ name = ('_' if cmb[0] else '_not_') + 'persistent_'
+ name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
+ name += '_online' if cmb[2] else '_offline'
+ name += '_shared' if cmb[3] else '_nonshared'
+ if cmb[4]:
+ name += '__pre_shutdown'
+
+ inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
+ *list(cmb))
+
+ for cmb in list(itertools.product((True, False), repeat=2)):
+ name = ('_' if cmb[0] else '_not_') + 'persistent_'
+ name += ('_' if cmb[1] else '_not_') + 'migbitmap'
+
+ inject_test_case(TestDirtyBitmapMigration, name,
+ 'do_test_migration_resume_source', *list(cmb))
+
+ iotests.main(
+ supported_fmts=['qcow2'],
+ supported_protocols=['file']
+ )
+
+
if __name__ == '__main__':
- iotests.main(supported_fmts=['qcow2'],
- supported_protocols=['file'])
+ main()
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 06/17] iotests/297: modify is_python_file to work from any CWD
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (4 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 05/17] iotests/migrate-bitmaps-test: delint John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 07/17] iotests/297: Add get_files() function John Snow
` (12 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Add a directory argument to is_python_file to allow it to work correctly
no matter what CWD we happen to run it from. This is done in
anticipation of running the iotests from another directory (./python/).
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 433b7323368..493dda17fb6 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -39,14 +39,16 @@ SKIP_FILES = (
)
-def is_python_file(filename):
- if not os.path.isfile(filename):
+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(filename) as f:
+ with open(filepath) as f:
try:
first_line = f.readline()
return re.match('^#!.*python', first_line) is not None
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 07/17] iotests/297: Add get_files() function
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (5 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 06/17] iotests/297: modify is_python_file to work from any CWD John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 08/17] iotests/297: Include sub-directories when finding tests to lint John Snow
` (11 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Split out file discovery into its own method to begin separating out the
"environment setup" and "test execution" phases.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/297 | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 493dda17fb6..0bc11958059 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -21,6 +21,7 @@ import re
import shutil
import subprocess
import sys
+from typing import List
import iotests
@@ -56,9 +57,15 @@ def is_python_file(filename: str, directory: str = '.') -> bool:
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 = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
- if is_python_file(filename)]
+ files = get_test_files()
iotests.logger.debug('Files to be checked:')
iotests.logger.debug(', '.join(sorted(files)))
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 08/17] iotests/297: Include sub-directories when finding tests to lint
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (6 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 07/17] iotests/297: Add get_files() function John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 09/17] iotests/297: Don't rely on distro-specific linter binaries John Snow
` (10 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Choosing to interpret the SKIP_FILES list as a list of filenames instead
of a list of paths, to keep things simple.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/297 | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 0bc11958059..665ac0aa361 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -40,13 +40,8 @@ SKIP_FILES = (
)
-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'):
+def is_python_file(filepath: str) -> bool:
+ if filepath.endswith('.py'):
return True
with open(filepath) as f:
@@ -58,10 +53,20 @@ def is_python_file(filename: str, directory: str = '.') -> bool:
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)
- ]
+ files = []
+
+ iotests.logger.debug("get_test_files(%s)", directory)
+ for dirent in os.scandir(directory):
+ if dirent.name in SKIP_FILES:
+ continue
+
+ relpath = os.path.join(directory, dirent.name)
+ if dirent.is_dir():
+ files.extend(get_test_files(relpath))
+ elif is_python_file(relpath):
+ files.append(relpath)
+
+ return files
def run_linters():
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 09/17] iotests/297: Don't rely on distro-specific linter binaries
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (7 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 08/17] iotests/297: Include sub-directories when finding tests to lint John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:59 ` Philippe Mathieu-Daudé
2021-07-20 17:33 ` [PATCH v2 10/17] iotests/297: Create main() function John Snow
` (9 subsequent siblings)
18 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
'pylint-3' is another Fedora-ism. Use "python3 -m pylint" or "python3 -m
mypy" to access these scripts instead. This style of invocation will
prefer the "correct" tool when run in a virtual environment.
Note that we still check for "pylint-3" before the test begins -- this
check is now "overly strict", but shouldn't cause anything that was
already running correctly to start failing.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 45 ++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 665ac0aa361..dc4460dd84b 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -87,8 +87,11 @@ def run_linters():
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)
+ subprocess.run(
+ ('python3', '-m', 'pylint', '--score=n', '--notes=FIXME,XXX', *files),
+ env=env,
+ check=False,
+ )
print('=== mypy ===')
sys.stdout.flush()
@@ -99,23 +102,27 @@ def run_linters():
# 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)
+ 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,
+ ),
+ env=env,
+ check=False,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ universal_newlines=True
+ )
if p.returncode != 0:
print(p.stdout)
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 09/17] iotests/297: Don't rely on distro-specific linter binaries
2021-07-20 17:33 ` [PATCH v2 09/17] iotests/297: Don't rely on distro-specific linter binaries John Snow
@ 2021-07-20 17:59 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 24+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-20 17:59 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
On 7/20/21 7:33 PM, John Snow wrote:
> 'pylint-3' is another Fedora-ism. Use "python3 -m pylint" or "python3 -m
> mypy" to access these scripts instead. This style of invocation will
> prefer the "correct" tool when run in a virtual environment.
>
> Note that we still check for "pylint-3" before the test begins -- this
> check is now "overly strict", but shouldn't cause anything that was
> already running correctly to start failing.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/297 | 45 ++++++++++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 10/17] iotests/297: Create main() function
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (8 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 09/17] iotests/297: Don't rely on distro-specific linter binaries John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 11/17] iotests/297: Separate environment setup from test execution John Snow
` (8 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Instead of running "run_linters" directly, create a main() function that
will be responsible for environment setup, leaving run_linters()
responsible only for execution of the linters.
(That environment setup will be moved over in forthcoming commits.)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index dc4460dd84b..7ef4cdc7615 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -128,8 +128,12 @@ def run_linters():
print(p.stdout)
-for linter in ('pylint-3', 'mypy'):
- if shutil.which(linter) is None:
- iotests.notrun(f'{linter} not found')
+def main() -> None:
+ for linter in ('pylint-3', 'mypy'):
+ if shutil.which(linter) is None:
+ iotests.notrun(f'{linter} not found')
-iotests.script_main(run_linters)
+ run_linters()
+
+
+iotests.script_main(main)
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 11/17] iotests/297: Separate environment setup from test execution
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (9 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 10/17] iotests/297: Create main() function John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 12/17] iotests/297: Add 'directory' argument to run_linters John Snow
` (7 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Move environment setup into main(), leaving pure test execution behind
in run_linters().
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 7ef4cdc7615..961c9170d21 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -21,7 +21,7 @@ import re
import shutil
import subprocess
import sys
-from typing import List
+from typing import List, Mapping, Optional
import iotests
@@ -69,24 +69,16 @@ def get_test_files(directory: str = '.') -> List[str]:
return files
-def run_linters():
- files = get_test_files()
-
- iotests.logger.debug('Files to be checked:')
- iotests.logger.debug(', '.join(sorted(files)))
+def run_linters(
+ files: List[str],
+ env: Optional[Mapping[str, str]] = None,
+) -> None:
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')
- try:
- env['PYTHONPATH'] += os.pathsep + qemu_module_path
- except KeyError:
- env['PYTHONPATH'] = qemu_module_path
subprocess.run(
('python3', '-m', 'pylint', '--score=n', '--notes=FIXME,XXX', *files),
env=env,
@@ -100,7 +92,6 @@ def run_linters():
# 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(
(
@@ -133,7 +124,22 @@ def main() -> None:
if shutil.which(linter) is None:
iotests.notrun(f'{linter} not found')
- run_linters()
+ files = get_test_files()
+
+ iotests.logger.debug('Files to be checked:')
+ iotests.logger.debug(', '.join(sorted(files)))
+
+ env = os.environ.copy()
+ 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
+
+ env['MYPYPATH'] = env['PYTHONPATH']
+
+ run_linters(files, env=env)
iotests.script_main(main)
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 12/17] iotests/297: Add 'directory' argument to run_linters
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (10 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 11/17] iotests/297: Separate environment setup from test execution John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 13/17] iotests/297: return error code from run_linters() John Snow
` (6 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Allow run_linters to work well if it's executed from a different
directory.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 961c9170d21..b2bf7928c5d 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -71,6 +71,7 @@ def get_test_files(directory: str = '.') -> List[str]:
def run_linters(
files: List[str],
+ directory: str = '.',
env: Optional[Mapping[str, str]] = None,
) -> None:
@@ -81,6 +82,7 @@ def run_linters(
# fixed (in tests, at least)
subprocess.run(
('python3', '-m', 'pylint', '--score=n', '--notes=FIXME,XXX', *files),
+ cwd=directory,
env=env,
check=False,
)
@@ -108,6 +110,7 @@ def run_linters(
'--namespace-packages',
filename,
),
+ cwd=directory,
env=env,
check=False,
stdout=subprocess.PIPE,
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 13/17] iotests/297: return error code from run_linters()
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (11 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 12/17] iotests/297: Add 'directory' argument to run_linters John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 14/17] iotests/297: split linters.py off from 297 John Snow
` (5 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
This turns run_linters() into a bit of a hybrid test; returning non-zero
on failed execution while also printing diffable information. This is
done for the benefit of the avocado simple test runner, which will soon
be attempting to execute this test from a different environment.
(Note: universal_newlines is added to the pylint invocation for type
consistency with the mypy run -- it's not strictly necessary, but it
avoids some typing errors caused by our re-use of the 'p' variable.)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/297 | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b2bf7928c5d..ca9ee72d2fa 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -73,19 +73,22 @@ def run_linters(
files: List[str],
directory: str = '.',
env: Optional[Mapping[str, str]] = None,
-) -> 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)
- subprocess.run(
+ 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()
@@ -118,9 +121,12 @@ def run_linters(
universal_newlines=True
)
+ ret += p.returncode
if p.returncode != 0:
print(p.stdout)
+ return ret
+
def main() -> None:
for linter in ('pylint-3', 'mypy'):
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 14/17] iotests/297: split linters.py off from 297
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (12 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 13/17] iotests/297: return error code from run_linters() John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 15/17] iotests/linters: Add entry point for Python CI linters John Snow
` (4 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Split the linter execution itself out from 297, leaving just environment
setup in 297. This is done so that non-iotest code can invoke the
linters without needing to worry about imports of unpackaged iotest
code.
Eventually, it should be possible to replace linters.py with mypy.ini
and pylintrc files that instruct these tools how to run properly in this
directory, but ... not yet, and not in this series.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/297 | 110 ++-----------------------------
tests/qemu-iotests/linters.py | 121 ++++++++++++++++++++++++++++++++++
2 files changed, 125 insertions(+), 106 deletions(-)
create mode 100755 tests/qemu-iotests/linters.py
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index ca9ee72d2fa..3d29af5b78a 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -17,115 +17,13 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import os
-import re
import shutil
-import subprocess
-import sys
-from typing import List, Mapping, Optional
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(filepath: str) -> bool:
- if filepath.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]:
- files = []
-
- iotests.logger.debug("get_test_files(%s)", directory)
- for dirent in os.scandir(directory):
- if dirent.name in SKIP_FILES:
- continue
-
- relpath = os.path.join(directory, dirent.name)
- if dirent.is_dir():
- files.extend(get_test_files(relpath))
- elif is_python_file(relpath):
- files.append(relpath)
-
- return files
-
-
-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
+# Looking for the list of excluded tests? See linters.py !
def main() -> None:
@@ -133,7 +31,7 @@ def main() -> None:
if shutil.which(linter) is None:
iotests.notrun(f'{linter} not found')
- files = get_test_files()
+ files = linters.get_test_files()
iotests.logger.debug('Files to be checked:')
iotests.logger.debug(', '.join(sorted(files)))
@@ -148,7 +46,7 @@ def main() -> None:
env['MYPYPATH'] = env['PYTHONPATH']
- run_linters(files, env=env)
+ linters.run_linters(files, env=env)
iotests.script_main(main)
diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
new file mode 100755
index 00000000000..e7e8baded02
--- /dev/null
+++ b/tests/qemu-iotests/linters.py
@@ -0,0 +1,121 @@
+# 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(filepath: str) -> bool:
+ if filepath.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]:
+ files = []
+
+ for dirent in os.scandir(directory):
+ if dirent.name in SKIP_FILES:
+ continue
+
+ relpath = os.path.join(directory, dirent.name)
+ if dirent.is_dir():
+ files.extend(get_test_files(relpath))
+ elif is_python_file(relpath):
+ files.append(relpath)
+
+ return files
+
+
+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
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 15/17] iotests/linters: Add entry point for Python CI linters
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (13 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 14/17] iotests/297: split linters.py off from 297 John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 16/17] python: Add iotest linters to test suite John Snow
` (3 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
Add a main() function to linters.py so that the Python CI infrastructure
has something it can run.
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 <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/linters.py | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
index e7e8baded02..242cb9c3e69 100755
--- a/tests/qemu-iotests/linters.py
+++ b/tests/qemu-iotests/linters.py
@@ -119,3 +119,16 @@ def run_linters(
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 16/17] python: Add iotest linters to test suite
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (14 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 15/17] iotests/linters: Add entry point for Python CI linters John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 17:33 ` [PATCH v2 17/17] iotests/linters: check mypy files all at once John Snow
` (2 subsequent siblings)
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
As a convenience, since iotests is an extremely prominent user of the
qemu.qmp and qemu.machine packages and already implements a linting
regime, run those tests as well so that it's very hard to miss
regressions caused by changes to the python library.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
python/tests/iotests.sh | 2 ++
1 file changed, 2 insertions(+)
create mode 100755 python/tests/iotests.sh
diff --git a/python/tests/iotests.sh b/python/tests/iotests.sh
new file mode 100755
index 00000000000..ec2fc58066d
--- /dev/null
+++ b/python/tests/iotests.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+PYTHONPATH=../tests/qemu-iotests/ python3 -m linters
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v2 17/17] iotests/linters: check mypy files all at once
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (15 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 16/17] python: Add iotest linters to test suite John Snow
@ 2021-07-20 17:33 ` John Snow
2021-07-20 19:17 ` [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
2021-08-24 15:13 ` Hanna Reitz
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 17:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa, John Snow
We can circumvent the '__main__' redefinition problem by passing
--scripts-are-modules. Take mypy out of the loop per-filename and check
everything in one go: it's quite a bit faster.
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/linters.py | 56 ++++++++++++++++-------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
index 242cb9c3e69..b95425e81aa 100755
--- a/tests/qemu-iotests/linters.py
+++ b/tests/qemu-iotests/linters.py
@@ -86,37 +86,33 @@ def run_linters(
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
- )
+ 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',
+ '--scripts-are-modules',
+ *files,
+ ),
+ 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)
+ ret += p.returncode
+ if p.returncode != 0:
+ print(p.stdout)
return ret
--
2.31.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (16 preceding siblings ...)
2021-07-20 17:33 ` [PATCH v2 17/17] iotests/linters: check mypy files all at once John Snow
@ 2021-07-20 19:17 ` John Snow
2021-08-24 15:13 ` Hanna Reitz
18 siblings, 0 replies; 24+ messages in thread
From: John Snow @ 2021-07-20 19:17 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
[-- Attachment #1: Type: text/plain, Size: 4719 bytes --]
I realize that some of this has been covered before:
https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg04221.html
Missed it at the time -- I suppose these patches never went in. I'll rebase
my series and front-load these patches.
--js
On Tue, Jul 20, 2021 at 1:33 PM John Snow <jsnow@redhat.com> wrote:
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-package-iotest
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/340144191
>
> Since iotests are such a heavy and prominent user of the Python qemu.qmp
> and qemu.machine packages, it would be convenient if the Python linting
> suite also checked this client for any possible regressions introduced
> by shifting around signatures, types, or interfaces in these packages.
>
> (We'd eventually find those problems when iotest 297 ran, but with
> increasing distance between Python development and Block development,
> the risk of an accidental breakage in this regard increases. I,
> personally, know to run iotests (and especially 297) after changing
> Python code, but not everyone in the future might. Plus, I am lazy, and
> I like only having to push one button.)
>
> Add the ability for the Python CI to run the iotest linters too, which
> means that the iotest linters would be checked against:
>
> - Python 3.6, using a frozen set of linting packages at their oldest
> supported versions, using 'pipenv'
> - Python 3.6 through Python 3.10 inclusive, using 'tox' and the latest
> versions of mypy/pylint that happen to be installed during test
> time. This CI test is allowed to fail with a warning, and can serve
> as a bellwether for when new incompatible changes may disrupt the
> linters. Testing against old and new Python interpreters alike can
> help surface incompatibility issues we may need to be aware of.)
>
> Here are example outputs of those CI jobs with this series applied:
> - "check-python-pipenv": https://gitlab.com/jsnow/qemu/-/jobs/1377735087
> - "check-python-tox": https://gitlab.com/jsnow/qemu/-/jobs/1377735088
>
> You can also run these same tests locally from ./python, plus one more:
>
> - "make check-dev" to test against whatever python you have.
> - "make check-pipenv", if you have Python 3.6 and pipenv installed.
> - "make check-tox", if you have Python 3.6 through Python 3.10 installed.
>
> See the old commit message for more sample output, etc.
>
> https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg07056.html
>
> V2:
> - Added patches 1-5 which do some more delinting.
> - Added patch 8, which scans subdirs for tests to lint.
> - Added patch 17, which improves the speed of mypy analysis.
> - Patch 14 is different because of the new patch 8.
>
> Unreviewed patches:
>
> [01] iotests-use-with-statement-for # [SOB] JS
> [02] iotests-use-subprocess.devnull # [SOB] JS
> [03] iotests-mirror-top-perms # [SOB] JS
> [04] iotests-migrate-bitmaps # [SOB] JS
> [05] iotests-migrate-bitmaps-test # [SOB] JS
> [07] iotests-297-add-get_files # [SOB] JS
> [08] wip-make-the-test-finding # [SOB] JS
> [14] iotests-297-split-linters-py # [SOB] JS
> [17] iotests-297-check-mypy-files # [SOB] JS
>
> --js
>
> John Snow (17):
> iotests: use with-statement for open() calls
> iotests: use subprocess.DEVNULL instead of open("/dev/null")
> iotests/mirror-top-perms: Adjust import paths
> iotests/migrate-bitmaps-postcopy-test: declare instance variables
> iotests/migrate-bitmaps-test: delint
> iotests/297: modify is_python_file to work from any CWD
> iotests/297: Add get_files() function
> iotests/297: Include sub-directories when finding tests to lint
> iotests/297: Don't rely on distro-specific linter binaries
> iotests/297: Create main() function
> iotests/297: Separate environment setup from test execution
> iotests/297: Add 'directory' argument to run_linters
> iotests/297: return error code from run_linters()
> iotests/297: split linters.py off from 297
> iotests/linters: Add entry point for Python CI linters
> python: Add iotest linters to test suite
> iotests/linters: check mypy files all at once
>
> python/tests/iotests.sh | 2 +
> tests/qemu-iotests/297 | 80 ++---------
> tests/qemu-iotests/iotests.py | 21 +--
> tests/qemu-iotests/linters.py | 130 ++++++++++++++++++
> .../tests/migrate-bitmaps-postcopy-test | 3 +
> tests/qemu-iotests/tests/migrate-bitmaps-test | 70 +++++-----
> tests/qemu-iotests/tests/mirror-top-perms | 7 +-
> 7 files changed, 198 insertions(+), 115 deletions(-)
> create mode 100755 python/tests/iotests.sh
> create mode 100755 tests/qemu-iotests/linters.py
>
> --
> 2.31.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 6168 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI
2021-07-20 17:33 [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
` (17 preceding siblings ...)
2021-07-20 19:17 ` [PATCH v2 00/17] python/iotests: Run iotest linters during Python CI John Snow
@ 2021-08-24 15:13 ` Hanna Reitz
18 siblings, 0 replies; 24+ messages in thread
From: Hanna Reitz @ 2021-08-24 15:13 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Eduardo Habkost,
qemu-block, Markus Armbruster, Max Reitz, Cleber Rosa
On 20.07.21 19:33, John Snow wrote:
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-package-iotest
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/340144191
I’ll take the liberty of applying patches 1 and 2 to my block-next
branch, because, well, they fix some of the 297 errors I’m seeing with
an updated pylint.
(There’s more still, namely some unspecified-encoding errors, one
use-dict-literal, and one use-list-literal, but I don’t think there are
patches for that in this series, so I guess I’ll have to have a go
myself...)
To make this formal:
Thanks, I’ve applied patches 1 and 2 to my block-next branch:
https://github.com/XanClic/qemu/commits/block-next
Hanna
^ permalink raw reply [flat|nested] 24+ messages in thread