* Re: Running 297 from GitLab CI
2021-10-01 8:21 ` Kevin Wolf
@ 2021-10-01 19:24 ` John Snow
2021-10-01 20:36 ` Eric Blake
1 sibling, 0 replies; 4+ messages in thread
From: John Snow @ 2021-10-01 19:24 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Hanna Reitz, qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]
On Fri, Oct 1, 2021 at 4:21 AM Kevin Wolf <kwolf@redhat.com> wrote:
> Am 30.09.2021 um 23:28 hat John Snow geschrieben:
> > Hiya, I was talking this over with Hanna in review to '[PATCH v3 00/16]
> > python/iotests: Run iotest linters during Python CI' [1] and I have some
> > doubt about what you'd personally like to see happen, here.
> >
> > In a nutshell, I split out 'linters.py' from 297 and keep all of the
> > iotest-bits in 297 and all of the generic "run the linters" bits in
> > linters.py, then I run linters.py from the GitLab python CI jobs.
> >
> > I did this so that iotest #297 would continue to work exactly as it had,
> > but trying to serve "two masters" in the form of two test suites means
> some
> > non-beautiful design decisions. Hanna suggested we just outright drop
> test
> > 297 to possibly improve the factoring of the tests.
> >
> > I don't want to do that unless you give it the go-ahead, though. I wanted
> > to hear your feelings on if we still want to keep 297 around or not.
>
> My basic requirement is that the checks are run somewhere in my local
> testing before I prepare a pull request. This means it could be done by
> iotests in any test that runs for -raw or -qcow2, or in 'make check'.
>
> So if you have a replacement somewhere in 'make check', dropping 297 is
> fine with me. If I have to run something entirely different, you may
> need to invest a bit more effort to convince me. ;-)
>
>
I love a good set of solid criteria ;-)
Understood! At the moment it *would* require a separate invocation. The
Python tests that run under CI generally set up their own environments to
ensure they'll run with minimum fuss or intervention from humans, though
there is an invocation in that Makefile that performs no environment setup
whatsoever -- but since no automated test uses it, it's not really a big
problem if your environment is "wrong" for that one. (But that also makes
it useless for make check!) It's similar to how iotest 297 does not really
check to see what version of pylint or mypy you might have, so sometimes
that test fails if your environment isn't suitable. But that one isn't part
of 'make check' either, so this feels like a bridge we've never crossed
anywhere in the whole source tree.
I have so far abstained from introducing such a step into 'make check'
because it might require some additional engineering to ensure that I get
the temporary directories all correct, tolerate the different types of
builds we do, uses the correct Python interpreter for all steps, etc.
So for now I'll propose leaving 297 as-is for convenience, but I will start
working towards finding the right way to include those tests from 'make
check'. I think there's no barrier (other than subjectively, beauty) to
leaving both avenues in for running the test for the time being. Maybe I
will find a way to convince Hanna to accept the interim solution as "well,
not THAT ugly."
Thanks for your input!
--js
[-- Attachment #2: Type: text/html, Size: 3651 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Running 297 from GitLab CI
2021-10-01 8:21 ` Kevin Wolf
2021-10-01 19:24 ` John Snow
@ 2021-10-01 20:36 ` Eric Blake
1 sibling, 0 replies; 4+ messages in thread
From: Eric Blake @ 2021-10-01 20:36 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Hanna Reitz, John Snow, qemu-devel, qemu-block
On Fri, Oct 01, 2021 at 10:21:36AM +0200, Kevin Wolf wrote:
> Am 30.09.2021 um 23:28 hat John Snow geschrieben:
> > Hiya, I was talking this over with Hanna in review to '[PATCH v3 00/16]
> > python/iotests: Run iotest linters during Python CI' [1] and I have some
> > doubt about what you'd personally like to see happen, here.
> >
> > In a nutshell, I split out 'linters.py' from 297 and keep all of the
> > iotest-bits in 297 and all of the generic "run the linters" bits in
> > linters.py, then I run linters.py from the GitLab python CI jobs.
> >
> > I did this so that iotest #297 would continue to work exactly as it had,
> > but trying to serve "two masters" in the form of two test suites means some
> > non-beautiful design decisions. Hanna suggested we just outright drop test
> > 297 to possibly improve the factoring of the tests.
> >
> > I don't want to do that unless you give it the go-ahead, though. I wanted
> > to hear your feelings on if we still want to keep 297 around or not.
>
> My basic requirement is that the checks are run somewhere in my local
> testing before I prepare a pull request. This means it could be done by
> iotests in any test that runs for -raw or -qcow2, or in 'make check'.
>
> So if you have a replacement somewhere in 'make check', dropping 297 is
> fine with me. If I have to run something entirely different, you may
> need to invest a bit more effort to convince me. ;-)
I'll echo that sentiment - if it's easy to automate, we can run it
under 'make check', and then we don't need the duplication of also
running it under iotests (especially since it isn't "really" an
iotest, so much as a test that makes the rest of the iotests more
consistent). If it's harder to automate, or requires me to run one
more thing, then keeping it in iotests for the short term is not too
drastic of a request, so that I don't accidentally skip it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 4+ messages in thread