Re: [PATCH 1/6] tests/acceptance/virtio_seg_max_adjust: Only remove listed machines

[Wainer dos Santos Moschetta]

Hi Philippe,

That fixes one of the problems I mention in another email thread. I was 
working on a fix, so putting my hands off it. :)

Anyway, see some comments below.

On 1/22/20 8:32 PM, Philippe Mathieu-Daudé wrote:
> Do not remove unavailable machines, this fixes:
>
>    VirtioMaxSegSettingsCheck.test_machine_types: ERROR: list.remove(x): x not 
> in list (0.12 s)
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>   tests/acceptance/virtio_seg_max_adjust.py | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tests/acceptance/virtio_seg_max_adjust.py 
> b/tests/acceptance/virtio_seg_max_adjust.py
> index 5458573138..4a417b8ef5 100755
> --- a/tests/acceptance/virtio_seg_max_adjust.py
> +++ b/tests/acceptance/virtio_seg_max_adjust.py
> @@ -109,14 +109,15 @@ class VirtioMaxSegSettingsCheck(Test):
>           return False
>   
>       def test_machine_types(self):
> -        # collect all machine types except 'none', 'isapc', 'microvm'
> +        EXCLUDED_MACHINES = ['none', 'isapc', 'microvm']

I was going to suggest moving this constant declaration alongside the 
others (VIRTIO_SCSI_PROPS, VIRTIO_BLK_PROPS...). But I saw on patch 04 
that this variable can get updated, i.e. no longer it is a constant, so 
I think it could be an object attribute instead. My reasoning is: it is 
easier to figure out what to change (eventually) if it is an object 
attribute or module constant.

Also if you want to make it further flexible, you can use Avocado's 
parameters. Example:

excluded_machines = self.params.get('exclude_machines', default=['none', 
'isapc', 'microvm'])

I hope this helps.

- Wainer

> +        # collect all machine types except the ones in EXCLUDED_MACHINES
>           with QEMUMachine(self.qemu_bin) as vm:
>               vm.launch()
>               machines = [m['name'] for m in vm.command('query-machines')]
>               vm.shutdown()
> -        machines.remove('none')
> -        machines.remove('isapc')
> -        machines.remove('microvm')
> +        for m in EXCLUDED_MACHINES:
> +            if m in machines:
> +                machines.remove(m)
>   
>           for dev_type in DEV_TYPES:
>               # create the list of machine types and their parameters.