Re: [PATCH qemu-server v3 1/3] tests: improve multiarch build support by introducing local get_host_arch helper

[Fiona Ebner <f.ebner@proxmox.com>]

Am 05.02.26 um 3:19 PM schrieb Dominik Csapak:
> Since mocking imported functions have to be done for each package that
> imports them, we would have to mock get_host_arch for each package that
> imports it from PVE::Tools. Instead, introduce one local helper here
> that uses the function from PVE::Tools. This allows us to only need to
> override it one time.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

with two small comments below

> @@ -29,6 +30,11 @@ my $arch_to_qemu_binary = {
>      x86_64 => '/usr/bin/qemu-system-x86_64',
>  };
>  
> +# wrapper around the Tools helper, having it here makes it easier for testing

To be even clearer: "easier to mock for testing"

> +sub get_host_arch {
> +    return PVE::Tools::get_host_arch();
> +}
> +
>  sub get_command_for_arch($) {
>      my ($arch) = @_;
>      return '/usr/bin/kvm' if get_host_arch() eq $arch; # i.e. native arch
> diff --git a/src/test/run_config2command_tests.pl b/src/test/run_config2command_tests.pl
> index 4c872d1c..c20f2377 100755
> --- a/src/test/run_config2command_tests.pl
> +++ b/src/test/run_config2command_tests.pl
> @@ -253,9 +253,6 @@ $qemu_server_module->mock(
>      kernel_has_vhost_net => sub {
>          return 1; # TODO: make this per-test configurable?
>      },
> -    get_host_arch => sub() {
> -        return $current_test->{host_arch} // 'x86_64';
> -    },
>      get_iscsi_initiator_name => sub {
>          return 'iqn.1993-08.org.debian:01:aabbccddeeff';
>      },
> @@ -388,6 +385,9 @@ $pve_common_tools->mock(
>              },
>          );
>      },
> +    get_host_arch => sub() {

Pre-existing from before the move, but I think we should drop the
parentheses for consistency with other mocked functions

> +        return $current_test->{host_arch} // 'x86_64';
> +    },
>  );
>  
>  my $pve_cpuconfig;