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. Kevin