Skip to content
  1. Oct 14, 2021
    • John Snow's avatar
      iotests: [RFC] drop iotest 297 · c667a740
      John Snow authored 
      

(This is highlighting a what-if, which might make it clear why any
special infrastructure is still required at all. It's not intended to
actually be merged at this step -- running JUST the iotest linters from
e.g. 'make check' is not yet accommodated, so there's no suitable
replacement for 297 for block test authors.)

Drop 297. As a consequence, we no longer need to pass an environment
variable to the mypy/pylint invocations, so that can be dropped. We also
now no longer need to hide output-except-on-error, so that can be
dropped as well.

The only thing that necessitates any special running logic anymore is
the skip list and the python-test-detection code. Without those, we
could easily codify the tests as simply:

[pylint|mypy] *.py tests/*.py

... and drop this entire file. We're not quite there yet, though.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      c667a740
    • John Snow's avatar
      python: Add iotest linters to test suite · 43b3c09a
      John Snow authored 
      

Run mypy and pylint on the iotests files directly from the Python CI
test infrastructure. This ensures that any accidental breakages to the
qemu.[qmp|aqmp|machine|utils] packages will be caught by that test
suite.

It also ensures that these linters are run with well-known versions and
test against a wide variety of python versions, which helps to find
accidental cross-version python compatibility issues.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarHanna Reitz <hreitz@redhat.com>
      43b3c09a
    • John Snow's avatar
      iotests/linters: Add workaround for mypy bug #9852 · e5451eeb
      John Snow authored 
      This one is insidious: if you write an import as "from {namespace}
import {subpackage}" as mirror-top-perms (now) does, mypy will fail on
every-other invocation *if* the package being imported is a typed,
installed, namespace-scoped package.

Upsettingly, that's exactly what 'qemu.[aqmp|qmp|machine]' et al are in
the context of Python CI tests.

Now, I could just edit mirror-top-perms to avoid this invocation, but
since I tripped on a landmine, I might as well head it off at the pass
and make sure nobody else trips on that same landmine.

It seems to have something to do with the order in which files are
checked as well, meaning the random order in which set(os.listdir())
produces the list of files to test will cause problems intermittently
and not just strictly "every other run".

This will be fixed in mypy >= 0.920, which is not released yet. The
workaround for now is to disable incremental checking, which avoids the
issue.

Note: This workaround is not applied when running iotest 297 directly,
because the bug does not surface there! Given the nature of CI jobs not
starting with any stale cache to begin with, this really only has a
half-second impact on manual runs of the Python test suite when executed
directly by a developer on their local machine. The workaround may be
removed when the Python package requirements can stipulate mypy 0.920 or
higher, which can happen as soon as it is released. (Barring any
unforseen compatibility issues that 0.920 may bring with it.)


See also:
 https://github.com/python/mypy/issues/11010
 https://github.com/python/mypy/issues/9852



Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarHanna Reitz <hreitz@redhat.com>
      e5451eeb
    • John Snow's avatar
      iotests/linters: Add entry point for linting via Python CI · b93e4b2b
      John Snow authored 
      

We need at least a tiny little shim here to join test file discovery
with test invocation. This logic could conceivably be hosted somewhere
in python/, but I felt it was strictly the least-rude thing to keep the
test logic here in iotests/, even if this small function isn't itself an
iotest.

Note that we don't actually even need the executable bit here, we'll be
relying on the ability to run this module as a script using Python CLI
arguments. No chance it gets misunderstood as an actual iotest that way.

(It's named, not in tests/, doesn't have the execute bit, and doesn't
have an execution shebang.)

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      b93e4b2b
    • John Snow's avatar
      iotests: split linters.py out from 297 · 14b8d17c
      John Snow authored 
      

Now, 297 is just the iotests-specific incantations and linters.py is as
minimal as I can think to make it. The only remaining element in here
that ought to be configuration and not code is the list of skip files,
but they're still numerous enough that repeating them for mypy and
pylint configurations both would be ... a hassle.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      14b8d17c
    • John Snow's avatar
      iotests/297: split test into sub-cases · f61f66d2
      John Snow authored 
      

Take iotest 297's main() test function and split it into two sub-cases
that can be skipped individually. We can also drop custom environment
setup from the pylint test as it isn't needed.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      f61f66d2
    • John Snow's avatar
      iotests/297: update tool availability checks · 3abf80d6
      John Snow authored 
      

As mentioned in 'iotests/297: Don't rely on distro-specific linter
binaries', these checks are overly strict. Update them to be in-line
with how we actually invoke the linters themselves.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      3abf80d6
    • John Snow's avatar
      iotests/297: Change run_linter() to raise an exception on failure · b19eae12
      John Snow authored 
      

Instead of using a process return code as the python function return
value (or just not returning anything at all), allow run_linter() to
raise an exception instead.

The responsibility for printing output on error shifts from the function
itself to the caller, who will know best how to present/format that
information. (Also, "suppress_output" is now a lot more accurate of a
parameter name.)

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      b19eae12
    • John Snow's avatar
      iotests/297: refactor run_[mypy|pylint] as generic execution shim · 5bf9b4b8
      John Snow authored 
      

There's virtually nothing special here anymore; we can combine these
into a single, rather generic function.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      5bf9b4b8
    • John Snow's avatar
      iotests/297: Split run_linters apart into run_pylint and run_mypy · e9438539
      John Snow authored 
      

Move environment setup into main(), and split the actual linter
execution into run_pylint and run_mypy, respectively.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarHanna Reitz <hreitz@redhat.com>
      e9438539
    • John Snow's avatar
      iotests/297: Don't rely on distro-specific linter binaries · 953e436f
      John Snow authored 
      

'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. This is addressed by a
commit later in this series;
  'iotests/297: update tool availability checks'.

Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé's avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: default avatarHanna Reitz <hreitz@redhat.com>
      953e436f
    • John Snow's avatar
      iotests/297: Create main() function · 89bc862a
      John Snow authored 
      

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: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé's avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: default avatarHanna Reitz <hreitz@redhat.com>
      89bc862a
  3. Oct 13, 2021
  5. Oct 12, 2021