Re: [Qemu-devel] [PATCH 02/12] Makefile: Rules for docker testing

["Alex Bennée" <alex.bennee@linaro.org>]

Fam Zheng <famz@redhat.com> writes:

> This adds a group of make targets to run docker tests, all are available
> in source tree without running ./configure.
>
> The usage is shown by "make docker".
>
> Besides the fixed ones, dynamic targets for building each image and
> running each test in each image are generated automatically by make,
> scanning $(SRC_PATH)/tests/docker/ files with specific patterns.
>
> Alternative to manually list particular targets (docker-run-FOO@BAR)
> set, you can control which tests/images to run by filtering variables,
> TESTS= and IMAGES=, which are expressed in Makefile pattern syntax,
> "foo% %bar ...". For example:
>
>     $ make docker-run IMAGES="ubuntu fedora"

This could do with repeating in the docs so that you don't need to
search commits for example invocations.

>
> Unfortunately, it's impossible to propagate "-j $JOBS" into make in
> containers, however since each combination is made a first class target
> is the top Makefile, "make -j$N docker-run" still parallels the tests
> coarsely.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  Makefile                      |  4 +-
>  tests/docker/Makefile.include | 88 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 tests/docker/Makefile.include
>
> diff --git a/Makefile b/Makefile
> index d0de2d4..7da70f4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -6,7 +6,7 @@ BUILD_DIR=$(CURDIR)
>  # Before including a proper config-host.mak, assume we are in the source tree
>  SRC_PATH=.
>
> -UNCHECKED_GOALS := %clean TAGS cscope ctags
> +UNCHECKED_GOALS := %clean TAGS cscope ctags docker docker-%
>
>  # All following code might depend on configuration variables
>  ifneq ($(wildcard config-host.mak),)
> @@ -651,3 +651,5 @@ endif
>  # Include automatically generated dependency files
>  # Dependencies in Makefile.objs files come from our recursive subdir rules
>  -include $(wildcard *.d tests/*.d)
> +
> +include $(SRC_PATH)/tests/docker/Makefile.include
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> new file mode 100644
> index 0000000..ca84c35
> --- /dev/null
> +++ b/tests/docker/Makefile.include
> @@ -0,0 +1,88 @@
> +# Makefile for Docker tests
> +
> +$(if $(quiet-command),,$(eval include $(SRC_PATH)/rules.mak))
> +
> +.PHONY: docker docker-build docker-run docker-clean
> +
> +DOCKER_SUFFIX = .docker
> +
> +DOCKER_IMAGES := $(patsubst %$(DOCKER_SUFFIX),%, $(shell \
> +	ls $(SRC_PATH)/tests/docker/ | grep '$(DOCKER_SUFFIX)$$'))
> +
> +DOCKER_SCRIPTS := $(shell ls $(SRC_PATH)/tests/docker/ | grep '\.sh$$')
> +DOCKER_TESTS := $(filter test-%, $(DOCKER_SCRIPTS))
> +DOCKER_TOOLS := $(filter-out test-%, $(DOCKER_SCRIPTS))
> +
> +TESTS ?= %
> +IMAGES ?= %
> +
> +$(foreach i,$(DOCKER_IMAGES), \
> +	$(eval docker-build: docker-build-$i) \
> +	$(foreach t,$(DOCKER_SCRIPTS), \
> +		$(eval docker-build-$i docker-run-$t@$i: IMAGE=$i) \
> +		$(eval docker-run-$t@$i: SCRIPT=$t) \
> +		$(eval docker-run-$t@$i: docker-build-$i) \
> +		$(if $(filter test-%,$t), \
> +			$(eval docker-run: docker-run-$t@$i) \
> +			$(eval docker-run-$t@: docker-run-$t@$i) \
> +			$(eval docker-run-@$i: docker-run-$t@$i)) \
> +))
> +
> +docker:
> +	@echo 'Building QEMU and running tests or tools inside Docker containers'
> +	@echo
> +	@echo 'Available targets:'
> +	@echo
> +	@echo '    docker:              Print this help.'
> +	@echo '    docker-run:          Run all image/test combinations.'
> +	@echo '                         You can override the test cases to run by providing'
> +	@echo '                         TESTS="foo bar" in the make command, and the image set'
> +	@echo '                         by providing IMAGES="baz qux".'
> +	@echo '    docker-clean:        Kill and remove residual docker testing containers.'
> +	@echo '    docker-build:        Build all images.'
> +	@echo '    docker-build-IMG:    Build image "IMG".'
> +	@echo '                         "IMG" is one of the listed image name."'
> +	@echo '    docker-run-FOO@BAR:  Run "FOO" in container "BAR".'
> +	@echo '                         "FOO" must be one of the listed test/tool name."'
> +	@echo '                         "BAR" must be one of the listed image name."'
> +	@echo '    docker-run-FOO@:     Run "FOO" in all containers.'
> +	@echo '                         "FOO" must be one of the listed test name."'
> +	@echo '    docker-run-@BAR:     Run all tests in container "BAR".'
> +	@echo '                         "BAR" must be one of the listed image name."'
> +	@echo
> +	@echo 'Available images:'
> +	@echo '    $(DOCKER_IMAGES)'
> +	@echo
> +	@echo 'Available tests:'
> +	@echo '    $(DOCKER_TESTS)'
> +	@echo
> +	@echo 'Available tools:'
> +	@echo '    $(DOCKER_TOOLS)'

I commend the addition of help ;-)

Perhaps replace FOO and BAR with TEST and IMAGE to cement the actual
rather than the abstract in the readers mind.

Also I expected this to work:

$ make docker-run-basic@fedora
Invalid target
make: *** [docker-run-basic@fedora] Error 1

The help gives:

  Available images:
      centos6 fedora ubuntu

  Available tests:
      test-basic.sh test-clang.sh test-mingw.sh

  Available tools:
      travis.sh

The test and .sh seem superfluous to the user wanting to invoke things.
We know they are tests and the implementation detail of the shell should
be irrelevant to the user.


> +
> +docker-build-%:
> +	@if test -z "$(IMAGE)"; then echo "Invalid target"; exit 1; fi
> +	$(if $(filter $(IMAGES),$(IMAGE)), $(call quiet-command,\
> +		$(SRC_PATH)/tests/docker/docker_build qemu:$(IMAGE) \
> +			$(SRC_PATH)/tests/docker/$(IMAGE).docker \
> +			$(if $V,-v,), "  BUILD $(IMAGE)"))
> +
> +docker-run-%:
> +	@if test -z "$(IMAGE)" || test -z "$(SCRIPT)"; \
> +		then echo "Invalid target"; exit 1; \
> +	fi
> +	$(if $(filter $(TESTS),$(SCRIPT)),$(if $(filter $(IMAGES),$(IMAGE)), \
> +		$(call quiet-command,\
> +			$(SRC_PATH)/tests/docker/docker_run \
> +				--privileged -t --rm --net=none \
> +				-v $$(realpath $(SRC_PATH)):/var/tmp/qemu \
> +				-e QEMU_SRC=/var/tmp/qemu \
> +				-e V=$(V) \
> +				-v /var/tmp/qemu-docker-ccache:/var/tmp/ccache \
> +				-e CCACHE_DIR=/var/tmp/ccache \
> +				qemu:$(IMAGE) \
> +				/var/tmp/qemu/tests/docker/run \
> +				/var/tmp/qemu/tests/docker/$(SCRIPT); \
> +			, "  RUN $(SCRIPT) in $(IMAGE)")))
> +
> +docker-clean:
> +	$(call quiet-command, $(SRC_PATH)/tests/docker/docker_clean)

Otherwise looking good. Thanks!

--
Alex Bennée