| Message ID | 20211019144918.3159078-13-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | python/iotests: Run iotest linters during Python CI | expand |
On 19.10.21 16:49, 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 <jsnow@redhat.com> > --- > tests/qemu-iotests/linters.py | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) Reviewed-by: Hanna Reitz <hreitz@redhat.com>
On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz <hreitz@redhat.com> wrote: > On 19.10.21 16:49, 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 <jsnow@redhat.com> > > --- > > tests/qemu-iotests/linters.py | 27 +++++++++++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > Reviewed-by: Hanna Reitz <hreitz@redhat.com> > > Thanks! I'll endeavor to try and clean up the list of exempted files to continue cleaning up this mess, but it's not a top priority right now. I'll put it on the backburner after I finish typing the QAPI generator. A lot of the weird compatibility goop will go away over time as I consolidate more of the venv logic. (I think this series is good to go, now? I think it could be applied in any order vs my other series. If you want, if/when you give the go-ahead for the other series, I could just stage them both myself and make sure they work well together and save you the headache.) --js
On Tue, Oct 26, 2021 at 2:36 PM John Snow <jsnow@redhat.com> wrote: > > > On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz <hreitz@redhat.com> wrote: > >> On 19.10.21 16:49, 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 <jsnow@redhat.com> >> > --- >> > tests/qemu-iotests/linters.py | 27 +++++++++++++++++++++++++++ >> > 1 file changed, 27 insertions(+) >> >> Reviewed-by: Hanna Reitz <hreitz@redhat.com> >> >> > Thanks! I'll endeavor to try and clean up the list of exempted files to > continue cleaning up this mess, but it's not a top priority right now. I'll > put it on the backburner after I finish typing the QAPI generator. A lot of > the weird compatibility goop will go away over time as I consolidate more > of the venv logic. > > (I think this series is good to go, now? I think it could be applied in > any order vs my other series. If you want, if/when you give the go-ahead > for the other series, I could just stage them both myself and make sure > they work well together and save you the headache.) > Update: I pre-emptively staged both series (the iotests one first, followed by the AQMP one) to jsnow/python and verified that all of the python tests pass for each commit between: [14] python-add-iotest-linters-to # python: Add iotest linters to test suite ... [22] python-iotests-replace-qmp # python, iotests: replace qmp with aqmp and I'm running the CI on all of that now at https://gitlab.com/jsnow/qemu/-/pipelines/396002744 (I just wanted to double-check they didn't conflict with each other in any unanticipated ways. Let me know if I should send the PR or if that'll just create hassle for you.) --js
On 26.10.21 21:45, John Snow wrote: > > > On Tue, Oct 26, 2021 at 2:36 PM John Snow <jsnow@redhat.com> wrote: > > > > On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz <hreitz@redhat.com> wrote: > > On 19.10.21 16:49, 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 <jsnow@redhat.com> > > --- > > tests/qemu-iotests/linters.py | 27 +++++++++++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > Reviewed-by: Hanna Reitz <hreitz@redhat.com> > > > Thanks! I'll endeavor to try and clean up the list of exempted > files to continue cleaning up this mess, but it's not a top > priority right now. I'll put it on the backburner after I finish > typing the QAPI generator. A lot of the weird compatibility goop > will go away over time as I consolidate more of the venv logic. > > (I think this series is good to go, now? I think it could be > applied in any order vs my other series. If you want, if/when you > give the go-ahead for the other series, I could just stage them > both myself and make sure they work well together and save you the > headache.) > > > Update: I pre-emptively staged both series (the iotests one first, > followed by the AQMP one) to jsnow/python and verified that all of the > python tests pass for each commit between: > > [14] python-add-iotest-linters-to # python: Add iotest linters to > test suite > ... > [22] python-iotests-replace-qmp # python, iotests: replace qmp > with aqmp > > and I'm running the CI on all of that now at > https://gitlab.com/jsnow/qemu/-/pipelines/396002744 > > (I just wanted to double-check they didn't conflict with each other in > any unanticipated ways. Let me know if I should send the PR or if > that'll just create hassle for you.) No, I’m all good with you taking (the blame for) them. :) Hanna
diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py index c515c7afe36..46c28fdcda0 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 @@ -74,3 +75,29 @@ def run_linter( stderr=subprocess.STDOUT if suppress_output else None, universal_newlines=True, ) + + +def main() -> None: + """ + Used by the Python CI system as an entry point to run these linters. + """ + def show_usage() -> None: + print(f"Usage: {sys.argv[0]} < --mypy | --pylint >", file=sys.stderr) + sys.exit(1) + + if len(sys.argv) != 2: + show_usage() + + files = get_test_files() + + if sys.argv[1] == '--pylint': + run_linter('pylint', files) + elif sys.argv[1] == '--mypy': + run_linter('mypy', files) + else: + print(f"Unrecognized argument: '{sys.argv[1]}'", file=sys.stderr) + show_usage() + + +if __name__ == '__main__': + main()
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 <jsnow@redhat.com> --- tests/qemu-iotests/linters.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)