diff mbox series

qemu-iotests test 297 tries to run python linters on editor backup files

Message ID CAFEAcA_y6xYLyK_qEjngtCm+Y5-Yuw-rqK2Qm0UhVAFHtp610w@mail.gmail.com
State New
Headers show
Series qemu-iotests test 297 tries to run python linters on editor backup files | expand

Commit Message

Peter Maydell Oct. 8, 2024, 4:31 p.m. UTC 
I made some changes to a block backend so I wanted to run the iotests.
I ran into an unrelated failure of iotest 297. The bulk of this
seems to be because the iotest tries to run on all files
in qemu-iotests, even on editor backups like in this case "040~"
(which is an old editor backup of 040). But there are also
some warnings about files that do exist in the tree and which
I haven't modified:

+tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
(no-self-use)
+tests/mirror-change-copy-mode:109:4: R0201: Method could be a
function (no-self-use)
+fat16.py:239:4: R0201: Method could be a function (no-self-use)

Q1: can we make this test not run the linters on editor backup files?
Q2: why do I see the errors above but they aren't in the reference file
output?


e104462:jammy:qemu-iotests$ ./check 297
QEMU          --
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-system-x86_64"
-nodefaults -display none -accel qtest
QEMU_IMG      --
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-img"
QEMU_IO       --
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-io"
--cache writeback --aio threads -f raw
QEMU_NBD      --
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-nbd"
IMGFMT        -- raw
IMGPROTO      -- file
PLATFORM      -- Linux/x86_64 e104462 5.15.0-89-generic
TEST_DIR      --
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch
SOCK_DIR      -- /tmp/qemu-iotests-0c1ft_vw
GDB_OPTIONS   --
VALGRIND_QEMU --
PRINT_QEMU_OUTPUT --

297   fail       [17:26:10] [17:26:23]   13.1s                output
mismatch (see /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch/raw-file-297/297.out.bad)
    [case not run] 'mypy' not found

Comments

John Snow Oct. 8, 2024, 4:50 p.m. UTC | #1 
On Tue, Oct 8, 2024, 12:31 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> I made some changes to a block backend so I wanted to run the iotests.
> I ran into an unrelated failure of iotest 297. The bulk of this
> seems to be because the iotest tries to run on all files
> in qemu-iotests, even on editor backups like in this case "040~"
> (which is an old editor backup of 040). But there are also
> some warnings about files that do exist in the tree and which
> I haven't modified:
>
> +tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
> (no-self-use)
> +tests/mirror-change-copy-mode:109:4: R0201: Method could be a
> function (no-self-use)
> +fat16.py:239:4: R0201: Method could be a function (no-self-use)
>
> Q1: can we make this test not run the linters on editor backup files?
>

Shouldn't be a problem. AFAIK we decide what to lint based on looking for
the shebang in the file and exclude a known-bad list, but we can exclude
the emacs confetti files too.

I'll fix it.

(Guess I haven't been editing the tests for a while... and nobody else has
mentioned it. oops.)

Q2: why do I see the errors above but they aren't in the reference file
> output?
>

You mean, why are there errors in files you haven't modified?

Very likely: pylint version differences. I've been meaning to remove iotest
297 for a long time, but when you run it directly through iotests (i.e. not
through the python directory tests, the ones that run on gitlab ci), the
linter versions are not controlled for. It's a remaining ugly spot of the
python consistency work. (sparing you the details on why but it's a known
thing I need to fix.)

In this case I bet "check-python-tox" (an optionally run, may-fail job) is
also failing on gitlab and I didn't notice yet.

for now (assuming my guesses above are right): I'll fix 297 to tolerate the
newest versions. As soon as I'm done my sphinx work, I'll pivot back to
finally adding a "check python" subtest to "make check" that *does* control
linter versions, and delete iotest 297.

Just in case my guesses are wrong, can you please go to your QEMU build
directory (post-configure) and type:

> source pyvenv/bin/activate.[whatever shell suffix you use]
> pylint --version
> deactivate

and let me know what version of pylint you have in the qemu build
environment?


>
> e104462:jammy:qemu-iotests$ ./check 297
> QEMU          --
>
> "/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-system-x86_64"
> -nodefaults -display none -accel qtest
> QEMU_IMG      --
> "/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-img"
> QEMU_IO       --
> "/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-io"
> --cache writeback --aio threads -f raw
> QEMU_NBD      --
> "/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/qemu-nbd"
> IMGFMT        -- raw
> IMGPROTO      -- file
> PLATFORM      -- Linux/x86_64 e104462 5.15.0-89-generic
> TEST_DIR      --
>
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch
> SOCK_DIR      -- /tmp/qemu-iotests-0c1ft_vw
> GDB_OPTIONS   --
> VALGRIND_QEMU --
> PRINT_QEMU_OUTPUT --
>
> 297   fail       [17:26:10] [17:26:23]   13.1s                output
> mismatch (see
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch/raw-file-297/297.out.bad)
>     [case not run] 'mypy' not found
>
> --- /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/qemu-iotests/297.out
> +++
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch/raw-file-297/297.out.bad
> @@ -1,2 +1,74 @@
>  === pylint ===
> +************* Module migrate-bitmaps-test
> +tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
> (no-self-use)
> +************* Module mirror-change-copy-mode
> +tests/mirror-change-copy-mode:109:4: R0201: Method could be a
> function (no-self-use)
> +************* Module fat16
> +fat16.py:239:4: R0201: Method could be a function (no-self-use)
> +************* Module 040~
> +040~:50:0: C0301: Line too long (85/79) (line-too-long)
> +040~:64:0: C0301: Line too long (86/79) (line-too-long)
> +040~:91:0: C0301: Line too long (138/79) (line-too-long)
> [PMM: deleted a lot more warnings about this editor backup file]
>  === mypy ===
> Some cases not run in:
> /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/qemu-iotests/297
> Failures: 297
> Failed 1 of 1 iotests
>
> thanks
> -- PMM
>

Thanks for the report.
--js

>
Peter Maydell Oct. 8, 2024, 5:03 p.m. UTC | #2 
On Tue, 8 Oct 2024 at 17:50, John Snow <jsnow@redhat.com> wrote:
>
>
>
> On Tue, Oct 8, 2024, 12:31 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> I made some changes to a block backend so I wanted to run the iotests.
>> I ran into an unrelated failure of iotest 297. The bulk of this
>> seems to be because the iotest tries to run on all files
>> in qemu-iotests, even on editor backups like in this case "040~"
>> (which is an old editor backup of 040). But there are also
>> some warnings about files that do exist in the tree and which
>> I haven't modified:
>>
>> +tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
>> (no-self-use)
>> +tests/mirror-change-copy-mode:109:4: R0201: Method could be a
>> function (no-self-use)
>> +fat16.py:239:4: R0201: Method could be a function (no-self-use)
>>
>> Q1: can we make this test not run the linters on editor backup files?
>
>
> Shouldn't be a problem. AFAIK we decide what to lint based on looking for the shebang in the file and exclude a known-bad list, but we can exclude the emacs confetti files too.
>
> I'll fix it.

Thanks!

>> Q2: why do I see the errors above but they aren't in the reference file
>> output?
>
>
> You mean, why are there errors in files you haven't modified?

Yes, and/or "why isn't the test forcing a pylint version?"
and/or "why is the test run by default if it depends on
the pylint version?" :-)

> Very likely: pylint version differences. I've been meaning to remove iotest 297 for a long time, but when you run it directly through iotests (i.e. not through the python directory tests, the ones that run on gitlab ci), the linter versions are not controlled for. It's a remaining ugly spot of the python consistency work. (sparing you the details on why but it's a known thing I need to fix.)
>
> In this case I bet "check-python-tox" (an optionally run, may-fail job) is also failing on gitlab and I didn't notice yet.

I kicked off a job by hand, and yes, it fails, but apparently
not for the same set of errors as the above...

https://gitlab.com/qemu-project/qemu/-/jobs/8009902380

> for now (assuming my guesses above are right): I'll fix 297 to tolerate the newest versions. As soon as I'm done my sphinx work, I'll pivot back to finally adding a "check python" subtest to "make check" that *does* control linter versions, and delete iotest 297.
>
> Just in case my guesses are wrong, can you please go to your QEMU build directory (post-configure) and type:
>
> > source pyvenv/bin/activate.[whatever shell suffix you use]
> > pylint --version
> > deactivate
>
> and let me know what version of pylint you have in the qemu build environment?

(pyvenv) e104462:jammy:x86-tgts$ pylint --version
pylint 2.12.2
astroid 2.9.3
Python 3.10.12 (main, Sep 11 2024, 15:47:36) [GCC 11.4.0]

(This is an Ubuntu 22.04 system.)

thanks
-- PMM
John Snow Oct. 8, 2024, 5:07 p.m. UTC | #3 
On Tue, Oct 8, 2024, 1:03 PM Peter Maydell <peter.maydell@linaro.org> wrote:

> On Tue, 8 Oct 2024 at 17:50, John Snow <jsnow@redhat.com> wrote:
> >
> >
> >
> > On Tue, Oct 8, 2024, 12:31 PM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >>
> >> I made some changes to a block backend so I wanted to run the iotests.
> >> I ran into an unrelated failure of iotest 297. The bulk of this
> >> seems to be because the iotest tries to run on all files
> >> in qemu-iotests, even on editor backups like in this case "040~"
> >> (which is an old editor backup of 040). But there are also
> >> some warnings about files that do exist in the tree and which
> >> I haven't modified:
> >>
> >> +tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
> >> (no-self-use)
> >> +tests/mirror-change-copy-mode:109:4: R0201: Method could be a
> >> function (no-self-use)
> >> +fat16.py:239:4: R0201: Method could be a function (no-self-use)
> >>
> >> Q1: can we make this test not run the linters on editor backup files?
> >
> >
> > Shouldn't be a problem. AFAIK we decide what to lint based on looking
> for the shebang in the file and exclude a known-bad list, but we can
> exclude the emacs confetti files too.
> >
> > I'll fix it.
>
> Thanks!
>
> >> Q2: why do I see the errors above but they aren't in the reference file
> >> output?
> >
> >
> > You mean, why are there errors in files you haven't modified?
>
> Yes, and/or "why isn't the test forcing a pylint version?"
> and/or "why is the test run by default if it depends on
> the pylint version?" :-)
>

"because it's always been like that and I've had difficulties fixing it",
mostly :)

qemu configure venv was the first step to finally fixing it, but I've run
into some troubles since but have been too busy with Sphinx junk.

I know it's bonkers, sorry!


> > Very likely: pylint version differences. I've been meaning to remove
> iotest 297 for a long time, but when you run it directly through iotests
> (i.e. not through the python directory tests, the ones that run on gitlab
> ci), the linter versions are not controlled for. It's a remaining ugly spot
> of the python consistency work. (sparing you the details on why but it's a
> known thing I need to fix.)
> >
> > In this case I bet "check-python-tox" (an optionally run, may-fail job)
> is also failing on gitlab and I didn't notice yet.
>
> I kicked off a job by hand, and yes, it fails, but apparently
> not for the same set of errors as the above...
>
> https://gitlab.com/qemu-project/qemu/-/jobs/8009902380
>
> > for now (assuming my guesses above are right): I'll fix 297 to tolerate
> the newest versions. As soon as I'm done my sphinx work, I'll pivot back to
> finally adding a "check python" subtest to "make check" that *does* control
> linter versions, and delete iotest 297.
> >
> > Just in case my guesses are wrong, can you please go to your QEMU build
> directory (post-configure) and type:
> >
> > > source pyvenv/bin/activate.[whatever shell suffix you use]
> > > pylint --version
> > > deactivate
> >
> > and let me know what version of pylint you have in the qemu build
> environment?
>
> (pyvenv) e104462:jammy:x86-tgts$ pylint --version
> pylint 2.12.2
> astroid 2.9.3
> Python 3.10.12 (main, Sep 11 2024, 15:47:36) [GCC 11.4.0]
>
> (This is an Ubuntu 22.04 system.)
>
> thanks
> -- PMM
>

Great, thanks. will work on this today and get everything green again.

>
diff mbox series

Patch

--- /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/qemu-iotests/297.out
+++ /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/x86-tgts/tests/qemu-iotests/scratch/raw-file-297/297.out.bad
@@ -1,2 +1,74 @@ 
 === pylint ===
+************* Module migrate-bitmaps-test
+tests/migrate-bitmaps-test:63:4: R0201: Method could be a function
(no-self-use)
+************* Module mirror-change-copy-mode
+tests/mirror-change-copy-mode:109:4: R0201: Method could be a
function (no-self-use)
+************* Module fat16
+fat16.py:239:4: R0201: Method could be a function (no-self-use)
+************* Module 040~
+040~:50:0: C0301: Line too long (85/79) (line-too-long)
+040~:64:0: C0301: Line too long (86/79) (line-too-long)
+040~:91:0: C0301: Line too long (138/79) (line-too-long)
[PMM: deleted a lot more warnings about this editor backup file]
 === mypy ===
Some cases not run in:
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/tests/qemu-iotests/297
Failures: 297
Failed 1 of 1 iotests

thanks
-- PMM