Re: [PULL 6/6] iotests: add backup-discard-source

[Vladimir Sementsov-Ogievskiy]

[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 <vsementsov@yandex-team.ru>
> > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> > Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> > Message-Id: <20240313152822.626493-6-vsementsov@yandex-team.ru>
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@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?


--
Best regards,
Vladimir