[PATCH-for-8.2?] tests/avocado: Make fetch_asset() inconditionally require a crypto hash

[PATCH-for-8.2?] tests/avocado: Make fetch_asset() inconditionally require a crypto hash

[Philippe Mathieu-Daudé]

In a perfect world we'd have reproducible tests,
but then we'd be sure we run the same binaries.
If a binary artifact isn't hashed, we have no idea
what we are running. Therefore enforce hashing for
all our artifacts.

With this change, unhashed artifacts produce:

  $ avocado run tests/avocado/multiprocess.py
   (1/2) tests/avocado/multiprocess.py:Multiprocess.test_multiprocess_x86_64:
   ERROR: QemuBaseTest.fetch_asset() missing 1 required positional argument: 'asset_hash' (0.19 s)

Inspired-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Based-on: <20231115145852.494052-1-thuth@redhat.com>
  "tests/avocado/multiprocess: Add asset hashes to silence warnings"
---
 tests/avocado/avocado_qemu/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index d71e989db6..304c428168 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -254,7 +254,7 @@ def setUp(self, bin_prefix):
             self.cancel("No QEMU binary defined or found in the build tree")
 
     def fetch_asset(self, name,
-                    asset_hash=None, algorithm=None,
+                    asset_hash, algorithm=None,
                     locations=None, expire=None,
                     find_only=False, cancel_on_missing=True):
         return super().fetch_asset(name,
-- 
2.41.0

Re: [PATCH-for-8.2?] tests/avocado: Make fetch_asset() inconditionally require a crypto hash

[Philippe Mathieu-Daudé]

On 15/11/23 16:32, Philippe Mathieu-Daudé wrote:
> In a perfect world we'd have reproducible tests,
> but then we'd be sure we run the same binaries.
> If a binary artifact isn't hashed, we have no idea
> what we are running. Therefore enforce hashing for
> all our artifacts.
> 
> With this change, unhashed artifacts produce:
> 
>    $ avocado run tests/avocado/multiprocess.py
>     (1/2) tests/avocado/multiprocess.py:Multiprocess.test_multiprocess_x86_64:
>     ERROR: QemuBaseTest.fetch_asset() missing 1 required positional argument: 'asset_hash' (0.19 s)
> 
> Inspired-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20231115145852.494052-1-thuth@redhat.com>
>    "tests/avocado/multiprocess: Add asset hashes to silence warnings"

and:

Based-on: <20231114143531.291820-1-thuth@redhat.com>
     "tests/avocado/intel_iommu: Add asset hashes to avoid warnings"

> ---
>   tests/avocado/avocado_qemu/__init__.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
> index d71e989db6..304c428168 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -254,7 +254,7 @@ def setUp(self, bin_prefix):
>               self.cancel("No QEMU binary defined or found in the build tree")
>   
>       def fetch_asset(self, name,
> -                    asset_hash=None, algorithm=None,
> +                    asset_hash, algorithm=None,
>                       locations=None, expire=None,
>                       find_only=False, cancel_on_missing=True):
>           return super().fetch_asset(name,

Re: [PATCH-for-8.2?] tests/avocado: Make fetch_asset() inconditionally require a crypto hash

[Thomas Huth]

On 15/11/2023 16.32, Philippe Mathieu-Daudé wrote:
> In a perfect world we'd have reproducible tests,
> but then we'd be sure we run the same binaries.
> If a binary artifact isn't hashed, we have no idea
> what we are running. Therefore enforce hashing for
> all our artifacts.
> 
> With this change, unhashed artifacts produce:
> 
>    $ avocado run tests/avocado/multiprocess.py
>     (1/2) tests/avocado/multiprocess.py:Multiprocess.test_multiprocess_x86_64:
>     ERROR: QemuBaseTest.fetch_asset() missing 1 required positional argument: 'asset_hash' (0.19 s)
> 
> Inspired-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20231115145852.494052-1-thuth@redhat.com>
>    "tests/avocado/multiprocess: Add asset hashes to silence warnings"
> ---
>   tests/avocado/avocado_qemu/__init__.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
> index d71e989db6..304c428168 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -254,7 +254,7 @@ def setUp(self, bin_prefix):
>               self.cancel("No QEMU binary defined or found in the build tree")
>   
>       def fetch_asset(self, name,
> -                    asset_hash=None, algorithm=None,
> +                    asset_hash, algorithm=None,
>                       locations=None, expire=None,
>                       find_only=False, cancel_on_missing=True):
>           return super().fetch_asset(name,

I think this already makes sense to avoid that those annoying warnings could 
get in again in the future!

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH-for-8.2?] tests/avocado: Make fetch_asset() inconditionally require a crypto hash

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

s/inconditionally/unconditionally/

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro