On Fri, Oct 1, 2021 at 4:21 AM Kevin Wolf <kw...@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
