[PATCH 0/4] python: update linting for new mypy/pylint releases

[PATCH 0/4] python: update linting for new mypy/pylint releases

[John Snow]

Various python tests in the "check-python-tox" test case on GitLab have
begun failing due to newer package versions. This patch set corrects
those issues and also improves the reliability of local developer tests
which may be using these tooling versions outside of GitLab pinned
version tests.

There are remaining issues with the "check-dev" test I have yet to
rectify, but appear unrelated to linter versions specifically and will
be handled separately.

As a result of this patch, the optionally-run and may-fail
"check-python-tox" test case on GitLab will become green again, and
local invocations of "make check-tox" in the python subdirectory will
also pass again. "check-python-minreqs" on GitLab and "make
check-minreqs" in the local developer environment were/are
unaffected. local iotest invocations for test case #297 ought to now
begin passing on developer workstations with bleeding-edge python
packages.

John Snow (4):
  iotests: reflow ReproducibleTestRunner arguments
  iotests: correct resultclass type in ReproducibleTestRunner
  python: disable too-many-positional-arguments warning
  python: silence pylint raising-non-exception error

 python/scripts/mkvenv.py      |  3 +++
 python/setup.cfg              |  1 +
 tests/qemu-iotests/iotests.py | 11 +++++++----
 tests/qemu-iotests/pylintrc   |  1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.47.0

[PATCH 1/4] iotests: reflow ReproducibleTestRunner arguments

[John Snow]

Trivial reflow to let the type names breathe.

(I need to add a longer type name.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ea48af4a7b6..673bbcd3563 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1614,10 +1614,13 @@ def write(self, arg=None):
         self.stream.write(arg)
 
 class ReproducibleTestRunner(unittest.TextTestRunner):
-    def __init__(self, stream: Optional[TextIO] = None,
-                 resultclass: Type[unittest.TestResult] =
-                 ReproducibleTestResult,
-                 **kwargs: Any) -> None:
+    def __init__(
+        self,
+        stream: Optional[TextIO] = None,
+        resultclass: Type[unittest.TestResult] =
+        ReproducibleTestResult,
+        **kwargs: Any
+    ) -> None:
         rstream = ReproducibleStreamWrapper(stream or sys.stdout)
         super().__init__(stream=rstream,           # type: ignore
                          descriptions=True,
-- 
2.47.0

[PATCH 2/4] iotests: correct resultclass type in ReproducibleTestRunner

[John Snow]

I have a vague memory that I suggested this base class to Vladimir and
said "Maybe someday it will break, and I'll just fix it then." Guess
that's today.

Fixes various mypy errors in the "make check-tox" python test for at
least Python3.8; seemingly requires a fairly modern mypy and/or Python
base version to trigger.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 673bbcd3563..19817c73530 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1617,7 +1617,7 @@ class ReproducibleTestRunner(unittest.TextTestRunner):
     def __init__(
         self,
         stream: Optional[TextIO] = None,
-        resultclass: Type[unittest.TestResult] =
+        resultclass: Type[unittest.TextTestResult] =
         ReproducibleTestResult,
         **kwargs: Any
     ) -> None:
-- 
2.47.0

[PATCH 3/4] python: disable too-many-positional-arguments warning

[John Snow]

Newest versions of pylint complain about specifically positional
arguments in addition to too many in general. We already disable the
general case, so silence this new warning too.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/setup.cfg            | 1 +
 tests/qemu-iotests/pylintrc | 1 +
 2 files changed, 2 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 3b4e2cc5501..cf5af7e6641 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -142,6 +142,7 @@ ignore_missing_imports = True
 disable=consider-using-f-string,
         consider-using-with,
         too-many-arguments,
+        too-many-positional-arguments,
         too-many-function-args,  # mypy handles this with less false positives.
         too-many-instance-attributes,
         no-member,  # mypy also handles this better.
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index 05b75ee59bf..c5f4833e458 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -13,6 +13,7 @@ disable=invalid-name,
         no-else-return,
         too-few-public-methods,
         too-many-arguments,
+        too-many-positional-arguments,
         too-many-branches,
         too-many-lines,
         too-many-locals,
-- 
2.47.0

[PATCH 4/4] python: silence pylint raising-non-exception error

[John Snow]

As of (at least) pylint 3.3.1, this code trips pylint up into believing
we are raising something other than an Exception. We are not: the first
two values may indeed be "None", but the last and final value must by
definition be a SystemExit exception.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/scripts/mkvenv.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index f2526af0a04..8ac5b0b2a05 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -379,6 +379,9 @@ def make_venv(  # pylint: disable=too-many-arguments
         try:
             builder.create(str(env_dir))
         except SystemExit as exc:
+            # pylint 3.3 bug:
+            # pylint: disable=raising-non-exception, raise-missing-from
+
             # Some versions of the venv module raise SystemExit; *nasty*!
             # We want the exception that prompted it. It might be a subprocess
             # error that has output we *really* want to see.
-- 
2.47.0

Re: [PATCH 0/4] python: update linting for new mypy/pylint releases

[Kevin Wolf]

Am 01.11.2024 um 18:36 hat John Snow geschrieben:
> Various python tests in the "check-python-tox" test case on GitLab have
> begun failing due to newer package versions. This patch set corrects
> those issues and also improves the reliability of local developer tests
> which may be using these tooling versions outside of GitLab pinned
> version tests.
> 
> There are remaining issues with the "check-dev" test I have yet to
> rectify, but appear unrelated to linter versions specifically and will
> be handled separately.
> 
> As a result of this patch, the optionally-run and may-fail
> "check-python-tox" test case on GitLab will become green again, and
> local invocations of "make check-tox" in the python subdirectory will
> also pass again. "check-python-minreqs" on GitLab and "make
> check-minreqs" in the local developer environment were/are
> unaffected. local iotest invocations for test case #297 ought to now
> begin passing on developer workstations with bleeding-edge python
> packages.
> 
> John Snow (4):
>   iotests: reflow ReproducibleTestRunner arguments
>   iotests: correct resultclass type in ReproducibleTestRunner
>   python: disable too-many-positional-arguments warning
>   python: silence pylint raising-non-exception error

Thanks, applied to the block branch.

(Yes, of course I had to wait until I ran into the problem patch 2 fixes
myself, and after figuring out the fix from the incomprehensible error
message, I found that this series already contains it.)

Kevin

Re: [PATCH 0/4] python: update linting for new mypy/pylint releases

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

On Wed, Nov 6, 2024 at 4:57 AM Kevin Wolf <kwolf@redhat.com> wrote:

> Am 01.11.2024 um 18:36 hat John Snow geschrieben:
> > Various python tests in the "check-python-tox" test case on GitLab have
> > begun failing due to newer package versions. This patch set corrects
> > those issues and also improves the reliability of local developer tests
> > which may be using these tooling versions outside of GitLab pinned
> > version tests.
> >
> > There are remaining issues with the "check-dev" test I have yet to
> > rectify, but appear unrelated to linter versions specifically and will
> > be handled separately.
> >
> > As a result of this patch, the optionally-run and may-fail
> > "check-python-tox" test case on GitLab will become green again, and
> > local invocations of "make check-tox" in the python subdirectory will
> > also pass again. "check-python-minreqs" on GitLab and "make
> > check-minreqs" in the local developer environment were/are
> > unaffected. local iotest invocations for test case #297 ought to now
> > begin passing on developer workstations with bleeding-edge python
> > packages.
> >
> > John Snow (4):
> >   iotests: reflow ReproducibleTestRunner arguments
> >   iotests: correct resultclass type in ReproducibleTestRunner
> >   python: disable too-many-positional-arguments warning
> >   python: silence pylint raising-non-exception error
>
> Thanks, applied to the block branch.
>

Thank you!


>
> (Yes, of course I had to wait until I ran into the problem patch 2 fixes
> myself, and after figuring out the fix from the incomprehensible error
> message, I found that this series already contains it.)
>

Sorry O:-)

The only issue remaining I'm aware of (besides the broader problem of
iotests not using pinned versions yet) is that "make check-dev" is failing
due to (maybe?) some changes in bleeding edge setuptools (?) that impact
how editable packages are installed. That test is only ever manually run
and developer-local though, so I think it shouldn't get in anyone's way but
mine.

If there's something else you're noticing, I'm unaware of it atm and you
should definitely let me know about it.


>
> Kevin
>
>

[-- Attachment #2: Type: text/html, Size: 3143 bytes --]