On 30.04.24 12:13, Kevin Wolf wrote:
Am 29.04.2024 um 20:39 hat Vladimir Sementsov-Ogievskiy geschrieben:
[Add John]
On 29.04.24 17:18, Richard Henderson wrote:
On 4/29/24 04:51, Vladimir Sementsov-Ogievskiy wrote:
Add test for a new backup option: discard-source.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Reviewed-by: Fiona Ebner <f.eb...@proxmox.com>
Tested-by: Fiona Ebner <f.eb...@proxmox.com>
Message-Id: <20240313152822.626493-6-vsement...@yandex-team.ru>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
.../qemu-iotests/tests/backup-discard-source | 152 ++++++++++++++++++
.../tests/backup-discard-source.out | 5 +
2 files changed, 157 insertions(+)
create mode 100755 tests/qemu-iotests/tests/backup-discard-source
create mode 100644 tests/qemu-iotests/tests/backup-discard-source.out
This fails check-python-minreqs
https://gitlab.com/qemu-project/qemu/-/jobs/6739551782
It appears to be a pylint issue.
tests/export-incoming-iothread:1:0: R0801: Similar lines in 2 files
==tests.backup-discard-source:[52:61]
==tests.copy-before-write:[54:63]
'file': {
'driver': iotests.imgfmt,
'file': {
'driver': 'file',
'filename': source_img,
}
},
'target': {
'driver': iotests.imgfmt, (duplicate-code)
Hmm. I don't think, that something should be changed in code.
splitting out part of this json object to a function? That's a test
for QMP command, and it's good that we see the command as is in one
place. I can swap some lines or rename variables to hack the linter,
but I'd prefer not doing so:)
For me that looks like a false-positive: yes it's a duplication, but
it should better be duplication, than complicating raw json objects by
reusing common parts.
What to do? As described in 22305c2a081b8b6 "python: Reduce strictness
of pylint's duplicate-code check", this check could be either be
disabled completely, or we can increase min-similarity-lines config
value.
I'd just disable it completely. Any thoughts?
I think it would make sense to treat tests differently from real
production code. In tests/, some duplication is bound to happen and
trying to unify things across test cases (which would mean moving to
iotests.py) often doesn't make sense. On the other hand, in python/ we
would probably want to unify duplicated code.
Agree. Happily, it turns out that tests already have separate tests/qemu-iotests/pylintrc
file, so that's not a problem. Still I decided to anot disable the check completely, but
just bump the limit, see "[PATCH] iotests/pylintrc: allow up to 10 similar
lines"
--
Best regards,
Vladimir
