[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Wainer dos Santos Moschetta
Subject: Re: [PATCH 1/6] tests/acceptance/virtio_seg_max_adjust: Only remove listed machines
Date: Thu, 23 Jan 2020 11:55:10 -0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

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 
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:
              machines = [m['name'] for m in vm.command('query-machines')]
-        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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]