[PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support

Cleber Rosa posted 13 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Cleber Rosa 1 month, 3 weeks ago
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/avocado/tuxrun_baselines.py | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tests/avocado/tuxrun_baselines.py b/tests/avocado/tuxrun_baselines.py
index 736e4aa289..bd02e88ed6 100644
--- a/tests/avocado/tuxrun_baselines.py
+++ b/tests/avocado/tuxrun_baselines.py
@@ -17,6 +17,7 @@
 from avocado_qemu import QemuSystemTest
 from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
 from avocado_qemu import wait_for_console_pattern
+from avocado.utils import archive
 from avocado.utils import process
 from avocado.utils.path import find_command
 
@@ -40,17 +41,12 @@ def get_tag(self, tagname, default=None):
 
         return default
 
+    @skipUnless(archive._probe_zstd_cmd(),
+                'Could not find "zstd", or it is not able to properly '
+                'decompress decompress the rootfs')
     def setUp(self):
         super().setUp()
 
-        # We need zstd for all the tuxrun tests
-        # See https://github.com/avocado-framework/avocado/issues/5609
-        zstd = find_command('zstd', False)
-        if zstd is False:
-            self.cancel('Could not find "zstd", which is required to '
-                        'decompress rootfs')
-        self.zstd = zstd
-
         # Process the TuxRun specific tags, most machines work with
         # reasonable defaults but we sometimes need to tweak the
         # config. To avoid open coding everything we store all these
@@ -99,8 +95,8 @@ def fetch_tuxrun_assets(self, csums=None, dt=None):
                                          asset_hash = isum,
                                          algorithm = "sha256")
 
-        cmd = f"{self.zstd} -d {disk_image_zst} -o {self.workdir}/rootfs.ext4"
-        process.run(cmd)
+        archive.extract(disk_image_zst, os.path.join(self.workdir,
+                                                     "rootfs.ext4"))
 
         if dt:
             dsum = csums.get(dt, None)
-- 
2.45.2
Re: [PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
On 26/7/24 15:44, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/avocado/tuxrun_baselines.py | 16 ++++++----------
>   1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/avocado/tuxrun_baselines.py b/tests/avocado/tuxrun_baselines.py
> index 736e4aa289..bd02e88ed6 100644
> --- a/tests/avocado/tuxrun_baselines.py
> +++ b/tests/avocado/tuxrun_baselines.py
> @@ -17,6 +17,7 @@
>   from avocado_qemu import QemuSystemTest
>   from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
>   from avocado_qemu import wait_for_console_pattern
> +from avocado.utils import archive
>   from avocado.utils import process
>   from avocado.utils.path import find_command
>   
> @@ -40,17 +41,12 @@ def get_tag(self, tagname, default=None):
>   
>           return default
>   
> +    @skipUnless(archive._probe_zstd_cmd(),

_probe_zstd_cmd() isn't public AFAICT, but more importantly
this doesn't work because this method has been added in v101.0.

> +                'Could not find "zstd", or it is not able to properly '
> +                'decompress decompress the rootfs')
>       def setUp(self):
>           super().setUp()
>   
> -        # We need zstd for all the tuxrun tests
> -        # See https://github.com/avocado-framework/avocado/issues/5609
> -        zstd = find_command('zstd', False)
> -        if zstd is False:
> -            self.cancel('Could not find "zstd", which is required to '
> -                        'decompress rootfs')
> -        self.zstd = zstd
> -
>           # Process the TuxRun specific tags, most machines work with
>           # reasonable defaults but we sometimes need to tweak the
>           # config. To avoid open coding everything we store all these
> @@ -99,8 +95,8 @@ def fetch_tuxrun_assets(self, csums=None, dt=None):
>                                            asset_hash = isum,
>                                            algorithm = "sha256")
>   
> -        cmd = f"{self.zstd} -d {disk_image_zst} -o {self.workdir}/rootfs.ext4"
> -        process.run(cmd)
> +        archive.extract(disk_image_zst, os.path.join(self.workdir,
> +                                                     "rootfs.ext4"))
>   
>           if dt:
>               dsum = csums.get(dt, None)
Re: [PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Cleber Rosa 1 month, 2 weeks ago
On Mon, Jul 29, 2024 at 10:39 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 26/7/24 15:44, Cleber Rosa wrote:
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/avocado/tuxrun_baselines.py | 16 ++++++----------
> >   1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/tests/avocado/tuxrun_baselines.py b/tests/avocado/tuxrun_baselines.py
> > index 736e4aa289..bd02e88ed6 100644
> > --- a/tests/avocado/tuxrun_baselines.py
> > +++ b/tests/avocado/tuxrun_baselines.py
> > @@ -17,6 +17,7 @@
> >   from avocado_qemu import QemuSystemTest
> >   from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
> >   from avocado_qemu import wait_for_console_pattern
> > +from avocado.utils import archive
> >   from avocado.utils import process
> >   from avocado.utils.path import find_command
> >
> > @@ -40,17 +41,12 @@ def get_tag(self, tagname, default=None):
> >
> >           return default
> >
> > +    @skipUnless(archive._probe_zstd_cmd(),
>
> _probe_zstd_cmd() isn't public AFAICT, but more importantly
> this doesn't work because this method has been added in v101.0.
>

While it's not the best practice to use private functions, I just
couldn't accept rewriting that for the skip condition.  I can make
sure future  versions (including 103.1) make it public.

Also, these patches count on the bump to 103.0 indeed.
Re: [PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
On 1/8/24 05:39, Cleber Rosa wrote:
> On Mon, Jul 29, 2024 at 10:39 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
>>
>> On 26/7/24 15:44, Cleber Rosa wrote:
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>    tests/avocado/tuxrun_baselines.py | 16 ++++++----------
>>>    1 file changed, 6 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tests/avocado/tuxrun_baselines.py b/tests/avocado/tuxrun_baselines.py
>>> index 736e4aa289..bd02e88ed6 100644
>>> --- a/tests/avocado/tuxrun_baselines.py
>>> +++ b/tests/avocado/tuxrun_baselines.py
>>> @@ -17,6 +17,7 @@
>>>    from avocado_qemu import QemuSystemTest
>>>    from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
>>>    from avocado_qemu import wait_for_console_pattern
>>> +from avocado.utils import archive
>>>    from avocado.utils import process
>>>    from avocado.utils.path import find_command
>>>
>>> @@ -40,17 +41,12 @@ def get_tag(self, tagname, default=None):
>>>
>>>            return default
>>>
>>> +    @skipUnless(archive._probe_zstd_cmd(),
>>
>> _probe_zstd_cmd() isn't public AFAICT, but more importantly
>> this doesn't work because this method has been added in v101.0.
>>
> 
> While it's not the best practice to use private functions, I just
> couldn't accept rewriting that for the skip condition.  I can make
> sure future  versions (including 103.1) make it public.
> 
> Also, these patches count on the bump to 103.0 indeed.

Then either mention it in the commit description to avoid wasting
time to developers cherry-picking / testing this single patch, or
move it after the version bump, avoiding bisectability issues.

Thanks,

Phil.

Re: [PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Cleber Rosa 1 month, 2 weeks ago
On Thu, Aug 1, 2024 at 8:59 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > Also, these patches count on the bump to 103.0 indeed.
>
> Then either mention it in the commit description to avoid wasting
> time to developers cherry-picking / testing this single patch, or
> move it after the version bump, avoiding bisectability issues.
>

You're right.  My bad.

Regards,
- Cleber.

> Thanks,
>
> Phil.
>
Re: [PATCH 10/13] tests/avocado/tuxrun_baselines.py: use Avocado's zstd support
Posted by Daniel P. Berrangé 1 month, 2 weeks ago
On Fri, Jul 26, 2024 at 09:44:35AM -0400, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/avocado/tuxrun_baselines.py | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|