On Mon, Apr 19, 2021 at 07:14:13PM +0200, Pavel Hrdina wrote:
> Having the function on mock library reflect more closely what we usually
> do in tests.
Yes & no - having it in testutilsqemu.c would make it available to all
QEMU tests, while domaincapsmock.c only makes it available from those
loading that mock.
Using testutilsqemu.c might be simpler if we need to expand what it
mocks, as we have one place to put all the hacks. It gets confusing
if we have 2 mocks both overriding the same function.
This isn't a NACK - just saying that I don't think the commit message
is a clear justification for the move. Guess I'd like an explanation
of why it should be restricted to just tests using domaincapsmock
vs any of those linking to testutilsqemu
>
> Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> ---
> tests/domaincapsmock.c | 16 ++++++++++++++++
> tests/testutilsqemu.c | 15 ---------------
> 2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
> index d81a898dc0..73690f0b9e 100644
> --- a/tests/domaincapsmock.c
> +++ b/tests/domaincapsmock.c
> @@ -16,6 +16,7 @@
>
> #include <config.h>
>
> +#include "virfile.h"
> #include "virhostcpu.h"
> #ifdef WITH_LIBXL
> # include "libxl/libxl_capabilities.h"
> @@ -40,3 +41,18 @@ virHostCPUGetMicrocodeVersion(virArch hostArch G_GNUC_UNUSED)
> {
> return 0;
> }
> +
> +char *
> +virFindFileInPath(const char *file)
> +{
> + if (g_str_has_prefix(file, "qemu-system") ||
> + g_str_equal(file, "qemu-kvm")) {
> + return g_strdup_printf("/usr/bin/%s", file);
> + }
> +
> + /* Nothing in tests should be relying on real files
> + * in host OS, so we return NULL to try to force
> + * an error in such a case
> + */
> + return NULL;
> +}
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 7451929807..0a2af5036e 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -117,21 +117,6 @@ static const char *qemu_default_ram_id[VIR_ARCH_LAST] = {
> [VIR_ARCH_SPARC] = "sun4m.ram",
> };
>
> -char *
> -virFindFileInPath(const char *file)
> -{
> - if (g_str_has_prefix(file, "qemu-system") ||
> - g_str_equal(file, "qemu-kvm")) {
> - return g_strdup_printf("/usr/bin/%s", file);
> - }
> -
> - /* Nothing in tests should be relying on real files
> - * in host OS, so we return NULL to try to force
> - * an error in such a case
> - */
> - return NULL;
> -}
> -
>
> virCapsHostNUMA *
> virCapabilitiesHostNUMANewHost(void)
> --
> 2.30.2
>
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 :|
