[PATCH 0/6] avocado_qemu: small fixes and tweaks

[PATCH 0/6] avocado_qemu: small fixes and tweaks

[Willian Rampazzo]

Standardize the super() calls according to PEP3135; fix import order based on
isort; fix and tweak the ssh `connect` method; explicitly return None on
`pick_default_qemu_bin` function to avoid linters R1710; and fix inheritance
order on `LinuxTest` to avoid future problems.

Note: borrowed https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg05117.html
      for completeness.

Cleber Rosa (1):
  Acceptance Tests: add standard clean up at test tearDown()

Willian Rampazzo (5):
  avocado_qemu: standardize supper() call following PEP3135
  avocado_qemu: fix import module based on isort
  avocado_qemu: tweak ssh connect method
  avocado_qemu: explicitly return None to avoid R1710
  avocado_qemu: fix inheritance order on LinuxTest class

 tests/acceptance/avocado_qemu/__init__.py | 30 +++++++++--------------
 1 file changed, 12 insertions(+), 18 deletions(-)

-- 
2.31.1

[PATCH 1/6] Acceptance Tests: add standard clean up at test tearDown()

[Willian Rampazzo]

From: Cleber Rosa <crosa@redhat.com>

The avocado.Test class, used as the basis of the avocado_qemu.Test
class, performs a clean of temporary directories up as part of its own
tearDown() implementation.

But the avocado_qemu.Test class is currently missing the same clean
up, as it implemented its own tearDown() method without resorting to
the upper class behavior.

This brings avocado_qemu.Test behavior in sync with the standard
avocado.Test behavior and prevents temporary directories from
cluttering the test results directory (unless instructed to do so with
Avocado's "--keep-tmp" option).

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Cleber Rosa <crosa@redhat.com>
[willianr: respin to new Python super format]
Signed-off-by: Willian Rampazzo <willianr@redhat.com>

---
v2
  - adjust super call to conform with PEP3135
---
 tests/acceptance/avocado_qemu/__init__.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 2c4fef3e14..d9e1b32aa1 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -276,6 +276,7 @@ def tearDown(self):
         for vm in self._vms.values():
             vm.shutdown()
         self._sd = None
+        super().tearDown()
 
     def fetch_asset(self, name,
                     asset_hash=None, algorithm=None,
-- 
2.31.1

[PATCH 2/6] avocado_qemu: standardize supper() call following PEP3135

[Willian Rampazzo]

PEP3135 states when calling super(), there is no need to use arguments.
This changes the calls on avocado_qemu to standardize according to
PEP3135 and avoid warnings from linters.

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index d9e1b32aa1..d2077d63cd 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -282,7 +282,7 @@ def fetch_asset(self, name,
                     asset_hash=None, algorithm=None,
                     locations=None, expire=None,
                     find_only=False, cancel_on_missing=True):
-        return super(Test, self).fetch_asset(name,
+        return super().fetch_asset(name,
                         asset_hash=asset_hash,
                         algorithm=algorithm,
                         locations=locations,
@@ -470,7 +470,7 @@ def _set_distro(self):
             self.distro.checksum = distro_checksum
 
     def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
-        super(LinuxTest, self).setUp()
+        super().setUp()
         self._set_distro()
         self.vm.add_args('-smp', '2')
         self.vm.add_args('-m', '1024')
-- 
2.31.1

Re: [PATCH 2/6] avocado_qemu: standardize supper() call following PEP3135

[Willian Rampazzo]

Ouch, s/supper/super/ in the email title.

On Mon, Sep 20, 2021 at 5:52 PM Willian Rampazzo <willianr@redhat.com> wrote:
>
> PEP3135 states when calling super(), there is no need to use arguments.
> This changes the calls on avocado_qemu to standardize according to
> PEP3135 and avoid warnings from linters.
>
> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index d9e1b32aa1..d2077d63cd 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -282,7 +282,7 @@ def fetch_asset(self, name,
>                      asset_hash=None, algorithm=None,
>                      locations=None, expire=None,
>                      find_only=False, cancel_on_missing=True):
> -        return super(Test, self).fetch_asset(name,
> +        return super().fetch_asset(name,
>                          asset_hash=asset_hash,
>                          algorithm=algorithm,
>                          locations=locations,
> @@ -470,7 +470,7 @@ def _set_distro(self):
>              self.distro.checksum = distro_checksum
>
>      def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
> -        super(LinuxTest, self).setUp()
> +        super().setUp()
>          self._set_distro()
>          self.vm.add_args('-smp', '2')
>          self.vm.add_args('-m', '1024')
> --
> 2.31.1
>
>

[PATCH 3/6] avocado_qemu: fix import module based on isort

[Willian Rampazzo]

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index d2077d63cd..edb9ed7485 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -12,19 +12,13 @@
 import os
 import shutil
 import sys
-import uuid
 import tempfile
+import uuid
 
 import avocado
-
-from avocado.utils import cloudinit
-from avocado.utils import datadrainer
-from avocado.utils import network
-from avocado.utils import ssh
-from avocado.utils import vmimage
+from avocado.utils import cloudinit, datadrainer, network, ssh, vmimage
 from avocado.utils.path import find_command
 
-
 #: The QEMU build root directory.  It may also be the source directory
 #: if building from the source dir, but it's safer to use BUILD_DIR for
 #: that purpose.  Be aware that if this code is moved outside of a source
@@ -42,11 +36,9 @@
 sys.path.append(os.path.join(SOURCE_DIR, 'python'))
 
 from qemu.machine import QEMUMachine
-from qemu.utils import (
-    get_info_usernet_hostfwd_port,
-    kvm_available,
-    tcg_available,
-)
+from qemu.utils import (get_info_usernet_hostfwd_port, kvm_available,
+                        tcg_available)
+
 
 def is_readable_executable_file(path):
     return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
-- 
2.31.1

[PATCH 4/6] avocado_qemu: tweak ssh connect method

[Willian Rampazzo]

The current implementation will crash if the connection fails as the
`time` module is not imported. This fixes the import problem and tweaks
the connection to wait progressively when the connection fails.

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index edb9ed7485..c3613f9262 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -13,6 +13,7 @@
 import shutil
 import sys
 import tempfile
+import time
 import uuid
 
 import avocado
@@ -305,8 +306,7 @@ def ssh_connect(self, username, credential, credential_is_key=True):
                 self.ssh_session.connect()
                 return
             except:
-                time.sleep(4)
-                pass
+                time.sleep(i)
         self.fail('ssh connection timeout')
 
     def ssh_command(self, command):
-- 
2.31.1

Re: [PATCH 4/6] avocado_qemu: tweak ssh connect method

[Philippe Mathieu-Daudé]

On 9/20/21 22:49, Willian Rampazzo wrote:
> The current implementation will crash if the connection fails as the
> `time` module is not imported. This fixes the import problem and tweaks
> the connection to wait progressively when the connection fails.
> 
> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> ---
>  tests/acceptance/avocado_qemu/__init__.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index edb9ed7485..c3613f9262 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -13,6 +13,7 @@
>  import shutil
>  import sys
>  import tempfile
> +import time
>  import uuid
>  
>  import avocado
> @@ -305,8 +306,7 @@ def ssh_connect(self, username, credential, credential_is_key=True):
>                  self.ssh_session.connect()
>                  return
>              except:
> -                time.sleep(4)

10 * 4 = 40

> -                pass
> +                time.sleep(i)

sum([0..10[) = 45

The described tweak. Almost the same, OK.

>          self.fail('ssh connection timeout')
>  
>      def ssh_command(self, command):
> 

Re: [PATCH 4/6] avocado_qemu: tweak ssh connect method

[Willian Rampazzo]

On Mon, Sep 27, 2021 at 11:12 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 9/20/21 22:49, Willian Rampazzo wrote:
> > The current implementation will crash if the connection fails as the
> > `time` module is not imported. This fixes the import problem and tweaks
> > the connection to wait progressively when the connection fails.
> >
> > Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> > ---
> >  tests/acceptance/avocado_qemu/__init__.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> > index edb9ed7485..c3613f9262 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -13,6 +13,7 @@
> >  import shutil
> >  import sys
> >  import tempfile
> > +import time
> >  import uuid
> >
> >  import avocado
> > @@ -305,8 +306,7 @@ def ssh_connect(self, username, credential, credential_is_key=True):
> >                  self.ssh_session.connect()
> >                  return
> >              except:
> > -                time.sleep(4)
>
> 10 * 4 = 40
>
> > -                pass
> > +                time.sleep(i)
>
> sum([0..10[) = 45
>
> The described tweak. Almost the same, OK.
>

The idea is that, hopefully, the connection will complete in one of
the first tries, so the overall wait time will be less than using a
fixed number. In the worst case, it will wait 5 seconds more than the
original code.

> >          self.fail('ssh connection timeout')
> >
> >      def ssh_command(self, command):
> >
>

[PATCH 5/6] avocado_qemu: explicitly return None to avoid R1710

[Willian Rampazzo]

The linter is complaining the `pick_default_qemu_bin` is not explicitly
returning None. Fix it to explicitly return None and avoid R1710
inconsistent-return-statements.

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index c3613f9262..35318ce2a9 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -72,6 +72,7 @@ def pick_default_qemu_bin(arch=None):
                                               qemu_bin_relative_path)
     if is_readable_executable_file(qemu_bin_from_bld_dir_path):
         return qemu_bin_from_bld_dir_path
+    return None
 
 
 def _console_interaction(test, success_message, failure_message,
-- 
2.31.1

[PATCH 6/6] avocado_qemu: fix inheritance order on LinuxTest class

[Willian Rampazzo]

Class hierarchy on Python is defined from right to left. Although the
current code is not harmful, let's fix it to avoid problems in the future.

Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 35318ce2a9..1841053e2c 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -424,7 +424,7 @@ def default_kernel_params(self):
         return self._info.get('kernel_params', None)
 
 
-class LinuxTest(Test, LinuxSSHMixIn):
+class LinuxTest(LinuxSSHMixIn, Test):
     """Facilitates having a cloud-image Linux based available.
 
     For tests that indend to interact with guests, this is a better choice
-- 
2.31.1

Re: [PATCH 0/6] avocado_qemu: small fixes and tweaks

[Philippe Mathieu-Daudé]

On 9/20/21 22:49, Willian Rampazzo wrote:
> Standardize the super() calls according to PEP3135; fix import order based on
> isort; fix and tweak the ssh `connect` method; explicitly return None on
> `pick_default_qemu_bin` function to avoid linters R1710; and fix inheritance
> order on `LinuxTest` to avoid future problems.
> 
> Note: borrowed https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg05117.html
>       for completeness.
> 
> Cleber Rosa (1):
>   Acceptance Tests: add standard clean up at test tearDown()
> 
> Willian Rampazzo (5):
>   avocado_qemu: standardize supper() call following PEP3135
>   avocado_qemu: fix import module based on isort
>   avocado_qemu: tweak ssh connect method
>   avocado_qemu: explicitly return None to avoid R1710
>   avocado_qemu: fix inheritance order on LinuxTest class

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 0/6] avocado_qemu: small fixes and tweaks

[Philippe Mathieu-Daudé]

On 9/20/21 22:49, Willian Rampazzo wrote:
> Standardize the super() calls according to PEP3135; fix import order based on
> isort; fix and tweak the ssh `connect` method; explicitly return None on
> `pick_default_qemu_bin` function to avoid linters R1710; and fix inheritance
> order on `LinuxTest` to avoid future problems.
> 
> Note: borrowed https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg05117.html
>       for completeness.
> 
> Cleber Rosa (1):
>   Acceptance Tests: add standard clean up at test tearDown()
> 
> Willian Rampazzo (5):
>   avocado_qemu: standardize supper() call following PEP3135
>   avocado_qemu: fix import module based on isort
>   avocado_qemu: tweak ssh connect method
>   avocado_qemu: explicitly return None to avoid R1710
>   avocado_qemu: fix inheritance order on LinuxTest class

Thanks, applied to my integration-testing tree.