Re: [PATCH v2 12/15] iotests/linters: Add entry point for linting via Python CI
On 26.10.21 21:45, John Snow wrote: On Tue, Oct 26, 2021 at 2:36 PM John Snow wrote: On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz 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 > --- > tests/qemu-iotests/linters.py | 27 +++ > 1 file changed, 27 insertions(+) Reviewed-by: Hanna Reitz 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
Re: [PATCH v2 12/15] iotests/linters: Add entry point for linting via Python CI
On Tue, Oct 26, 2021 at 2:36 PM John Snow wrote: > > > On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz 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 >> > --- >> > tests/qemu-iotests/linters.py | 27 +++ >> > 1 file changed, 27 insertions(+) >> >> Reviewed-by: Hanna Reitz >> >> > 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
Re: [PATCH v2 12/15] iotests/linters: Add entry point for linting via Python CI
On Tue, Oct 26, 2021 at 6:57 AM Hanna Reitz 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 > > --- > > tests/qemu-iotests/linters.py | 27 +++ > > 1 file changed, 27 insertions(+) > > Reviewed-by: Hanna Reitz > > 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
Re: [PATCH v2 12/15] iotests/linters: Add entry point for linting via Python CI
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 --- tests/qemu-iotests/linters.py | 27 +++ 1 file changed, 27 insertions(+) Reviewed-by: Hanna Reitz
[PATCH v2 12/15] iotests/linters: Add entry point for linting via Python CI
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
---
tests/qemu-iotests/linters.py | 27 +++
1 file changed, 27 insertions(+)
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()
--
2.31.1
