On Wed, Oct 13, 2021 at 8:11 AM Hanna Reitz <hre...@redhat.com> wrote:
> On 04.10.21 23:05, John Snow wrote:
> > 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: John Snow <js...@redhat.com>
> >
> > ---
> >
> > (1) I think that the test file discovery logic and skip list belong
> together,
> > and that those items belong in iotests/. I think they also belong in
> > whichever directory pylintrc and mypy.ini are in, also in iotests/.
>
> Agreed.
>
> > (2) Moving this logic into python/tests/ is challenging because I'd have
> > to import iotests code from elsewhere in the source tree, which just
> > inverts an existing problem I have been trying to rid us of --
> > needing to muck around with PYTHONPATH or sys.path hacking in python
> > scripts. I'm keen to avoid this.
>
> OK.
>
> > (3) If we moved all python tests into tests/ and gave them *.py
> > extensions, we wouldn't even need the test discovery functions
> > anymore, and all of linters.py could be removed entirely, including
> > this execution shim. We could rely on mypy/pylint's own file
> > discovery mechanisms at that point. More work than I'm up for with
> > just this series, but I could be coaxed into doing it if there was
> > some promise of not rejecting all that busywork ;)
>
> I believe the only real value this would gain is that the tests would
> have nicer names and we would have to delint them. If we find those
> goals to justify the effort, then we can put in the effort; and we can
> do that slowly, test by test. I don’t think we must do it now in one
> big lump just to get rid of the file discovery functions.
>
>
Yeah, I agree -- just do it over time and as-needed. I'm sure I will be
bothered by something-or-other sooner-or-later and I'll wind up doing it
anyway. Just maybe not this week!
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > tests/qemu-iotests/linters.py | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/linters.py
> b/tests/qemu-iotests/linters.py
> > index f6a2dc139fd..191df173064 100644
> > --- a/tests/qemu-iotests/linters.py
> > +++ b/tests/qemu-iotests/linters.py
> > @@ -16,6 +16,7 @@
> > import os
> > import re
> > import subprocess
> > +import sys
> > from typing import List, Mapping, Optional
> >
> >
> > @@ -81,3 +82,20 @@ def run_linter(
> >
> > return p.returncode
> >
> > +
> > +def main() -> int:
> > + """
> > + Used by the Python CI system as an entry point to run these linters.
> > + """
> > + files = get_test_files()
> > +
> > + if sys.argv[1] == '--pylint':
> > + return run_linter('pylint', files)
> > + elif sys.argv[1] == '--mypy':
> > + return run_linter('mypy', files)
>
> So I can run it as `python linters.py --pylint foo bar` and it won’t
> complain? :)
>
> I don’t feel like it’s important, but, well, it isn’t right either.
>
>
Alright. I hacked it together to be "minimal" in terms of SLOC, but I can
make it ... less minimal.
