diff mbox series

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

Message ID 20191104151323.9883-8-crosa@redhat.com
State New
Headers show
Series Acceptance test: Add "boot_linux" acceptance test | expand

Commit Message

Cleber Rosa Nov. 4, 2019, 3:13 p.m. UTC
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(-)

Comments

Wainer dos Santos Moschetta Nov. 7, 2019, 8:31 p.m. UTC | #1
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).

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).

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.

Thanks,

Wainer

>   	$(call quiet-command, \
>               $(TESTS_VENV_DIR)/bin/python -m avocado \
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
Cleber Rosa Nov. 15, 2019, 11:45 p.m. UTC | #2
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) \
diff mbox series

Patch

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)
 	$(call quiet-command, \
             $(TESTS_VENV_DIR)/bin/python -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \