Re: [PATCH v10 3/3] Acceptance tests: add make targets to download images

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3759 bytes --]

On Tue, Mar 17, 2020 at 06:36:18PM +0000, Alex Bennée wrote:
> 
> Cleber Rosa <crosa@redhat.com> writes:
> 
> > The newly introduced "boot linux" tests make use of Linux images that
> > are larger than usual, and fall into what Avocado calls "vmimages",
> > and can be referred to by name, version and architecture.
> >
> > The images can be downloaded automatically during the test. But, to
> > make for more reliable test results, this introduces a target that
> > will download the vmimages for the architectures that have been
> > configured and are available for the currently used distro (Fedora
> > 31).
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> > ---
> >  tests/Makefile.include | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 67e8fcddda..9c19229a06 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -20,6 +20,8 @@ check-help:
> >  	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
> >  	@echo " $(MAKE) check-clean          Clean the tests and related data"
> >  	@echo
> > +	@echo " $(MAKE) get-vm-images        Downloads all images used by acceptance tests, according to configured targets (~350 MB each, 1.5 GB max)"
> > +	@echo
> 
> I'm not overly enamoured with a super long line help for something that
> is a dependency anyway but whatever...
> 
>

I had a hard time choosing the right info to put there, and I didn't
want to break the style of the majority of help messages that use a
single line.  Anyway, we can always improve that.

> >  	@echo
> >  	@echo "The variable SPEED can be set to control the gtester speed setting."
> >  	@echo "Default options are -k and (for $(MAKE) V=1) --verbose; they can be"
> > @@ -889,7 +891,20 @@ $(TESTS_RESULTS_DIR):
> >  
> >  check-venv: $(TESTS_VENV_DIR)
> >  
> > -check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> > +FEDORA_31_ARCHES_CANDIDATES=$(patsubst ppc64,ppc64le,$(TARGETS))
> > +FEDORA_31_ARCHES := x86_64 aarch64 ppc64le s390x
> > +FEDORA_31_DOWNLOAD=$(filter $(FEDORA_31_ARCHES),$(FEDORA_31_ARCHES_CANDIDATES))
> > +
> > +# download one specific Fedora 31 image
> > +get-vm-image-fedora-31-%: $(check-venv)
> 
> Why $(check-venv) instead of check-venv
>

Good point.

> > +	$(call quiet-command, \
> > +             $(TESTS_VENV_DIR)/bin/python -m avocado vmimage get \
> > +             --distro=fedora --distro-version=31 --arch=$*)
> 
> Some short text for the operation would be nice (DNLD acceptance images?)
>

Another good point.

> > +
> > +# download all vm images, according to defined targets
> > +get-vm-images: $(check-venv) $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOWNLOAD))
> > +
> > +check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
> >  	$(call quiet-command, \
> >              $(TESTS_VENV_DIR)/bin/python -m avocado \
> >              --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
> > @@ -900,7 +915,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> >  
> >  # Consolidated targets
> >  
> > -.PHONY: check-block check-qapi-schema check-qtest check-unit check check-clean
> > +.PHONY: check-block check-qapi-schema check-qtest check-unit check check-clean get-vm-images
> >  check-qapi-schema: check-tests/qapi-schema/frontend check-tests/qapi-schema/doc-good.texi
> >  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
> >  ifeq ($(CONFIG_TOOLS),y)
> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 
>

Thanks!
- Cleber.

> -- 
> Alex Bennée
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]