Re: [PATCH v7 7/8] Acceptance tests: depend on qemu-img

From: Cleber Rosa <crosa@redhat.com>
To: Wainer dos Santos Moschetta <wainersm@redhat.com>
Cc: "Beraldo Leal" <bleal@redhat.com>,
	qemu-devel@nongnu.org, "Fabien Chouteau" <chouteau@adacore.com>,
	"KONRAD Frederic" <frederic.konrad@adacore.com>,
	"Hervé Poussineau" <hpoussin@reactos.org>,
	"Willian Rampazzo" <wrampazz@redhat.com>,
	qemu-ppc@nongnu.org,
	"Aleksandar Rikalo" <aleksandar.rikalo@rt-rk.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH v7 7/8] Acceptance tests: depend on qemu-img
Date: Fri, 15 Nov 2019 18:45:20 -0500	[thread overview]
Message-ID: <20191115234520.GJ19559@localhost.localdomain> (raw)
In-Reply-To: <3b075001-d6c5-3391-8fe8-515bbfb3d979@redhat.com>

On Thu, Nov 07, 2019 at 06:31:03PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/4/19 1:13 PM, Cleber Rosa wrote:
> > Tests using the avocado.utils.vmimage library make use of qemu-img,
> > and because it makes sense to use the version matching the rest of the
> > source code, let's make sure it gets built.
> > 
> > Its selection, instead of a possible qemu-img binary installed system
> > wide, is already dealt with by the change that adds the build dir to
> > the PATH during the test execution.
> > 
> > This is based on the same work for qemu-iotests, and suggested by its
> > author:
> > 
> >    - https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg00951.html
> > 
> > CC: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >   tests/Makefile.include | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 65e85f5275..559c3e6375 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -1174,7 +1174,7 @@ $(TESTS_RESULTS_DIR):
> >   check-venv: $(TESTS_VENV_DIR)
> > -check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> > +check-acceptance: check-venv $(TESTS_RESULTS_DIR) qemu-img$(EXESUF)
> 
> To be honest, I don't fell comfortable by the fact that the whole acceptance
> suite will depend on qemu-img which, in reality, is needed by only a sub-set
> of tests. Besides it, there might be some reason for someone to build QEMU
> with --disable-tools and this change will end up forcing the qemu-img built
> (of course if check-acceptance is issued).
>

Fair enough.

> What if we instead:
> 
> 1. Warn the users in case qemu tools weren't built. Alerting that qemu-img
> and friends will be picked up from system-wide (if any).
>

I'll propose on v8 that we try the locally built qemu-img, or fallback to the
system-wide install if available...

> 2. The tests that rely on avocado.utils.vmimage check for the presence of
> dependent tools, possible canceling itself on their lack. This may be done
> at test code level or perhaps using Avocado's tag mechanism + tweaking
> avocado_qemu.
>

... if not available, the test will automatically cancel because the
image creation (which depends on qemu-img) behaves like that.

Thanks!
- Cleber.

> Thanks,
> 
> Wainer
> 
> >   	$(call quiet-command, \
> >               $(TESTS_VENV_DIR)/bin/python -m avocado \
> >               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \