Message ID | CAFEAcA_y6xYLyK_qEjngtCm+Y5-Yuw-rqK2Qm0UhVAFHtp610w@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | qemu-iotests test 297 tries to run python linters on editor backup files | expand |
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 >
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
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. >
--- /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