diff mbox series

tests/vm: allow running tests in an unconfigured source tree

Message ID 20220614165126.1776413-1-pbonzini@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests/vm: allow running tests in an unconfigured source tree | expand

Commit Message

Paolo Bonzini June 14, 2022, 4:51 p.m. UTC
tests/vm/Makefile.include used to assume that it could run in an unconfigured
source tree, and Cirrus CI relies on that.  It was however broken by commit
f4c66f1705 ("tests: use tests/venv to run basevm.py-based scripts", 2022-06-06),
which co-opted the virtual environment being used by avocado tests
to also run the basevm.py tests.

For now, reintroduce the usage of qemu.qmp from the source directory, but
without the sys.path() hacks.  The CI configuration can be changed to
install the package via pip when qemu.qmp is removed from the source tree.

Cc: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/vm/Makefile.include | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Comments

John Snow June 15, 2022, 3:29 p.m. UTC | #1
On Tue, Jun 14, 2022 at 12:51 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> tests/vm/Makefile.include used to assume that it could run in an unconfigured
> source tree, and Cirrus CI relies on that.  It was however broken by commit
> f4c66f1705 ("tests: use tests/venv to run basevm.py-based scripts", 2022-06-06),
> which co-opted the virtual environment being used by avocado tests
> to also run the basevm.py tests.
>
> For now, reintroduce the usage of qemu.qmp from the source directory, but
> without the sys.path() hacks.  The CI configuration can be changed to
> install the package via pip when qemu.qmp is removed from the source tree.

Seems kinda fragile. If dependencies change etc they're not going to
get processed here unless someone notices and keeps it up to date
manually.
Can we change the Cirrus CI config to just create the venv and bypass
configure? (Like, just run mkvenv directly?)

Or ... actually, uh. what's the problem with running check-venv,
actually? I guess because of build system magic that requires a call
to configure and we can't do that for the Cirrus CI?

>
> Cc: John Snow <jsnow@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/vm/Makefile.include | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index 588bc999cc..5f5b1fbfe6 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -1,8 +1,17 @@
>  # Makefile for VM tests
>
> -.PHONY: vm-build-all vm-clean-all
> +# Hack to allow running in an unconfigured build tree
> +ifeq ($(wildcard $(SRC_PATH)/config-host.mak),)
> +VM_PYTHON = PYTHONPATH=$(SRC_PATH)/python /usr/bin/env python3
> +VM_VENV =
> +HOST_ARCH := $(shell uname -m)
> +else
> +VM_PYTHON = $(TESTS_PYTHON)
> +VM_VENV = check-venv
> +HOST_ARCH = $(ARCH)
> +endif
>
> -HOST_ARCH = $(if $(ARCH),$(ARCH),$(shell uname -m))
> +.PHONY: vm-build-all vm-clean-all
>
>  EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
>
> @@ -85,10 +94,10 @@ vm-clean-all:
>  $(IMAGES_DIR)/%.img:   $(SRC_PATH)/tests/vm/% \
>                         $(SRC_PATH)/tests/vm/basevm.py \
>                         $(SRC_PATH)/tests/vm/Makefile.include \
> -                       check-venv
> +                       $(VM_VENV)
>         @mkdir -p $(IMAGES_DIR)
>         $(call quiet-command, \
> -               $(TESTS_PYTHON) $< \
> +               $(VM_PYTHON) $< \
>                 $(if $(V)$(DEBUG), --debug) \
>                 $(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \
>                 $(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
> @@ -100,11 +109,10 @@ $(IMAGES_DIR)/%.img:      $(SRC_PATH)/tests/vm/% \
>                 --build-image $@, \
>                 "  VM-IMAGE $*")
>
> -
>  # Build in VM $(IMAGE)
> -vm-build-%: $(IMAGES_DIR)/%.img check-venv
> +vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
>         $(call quiet-command, \
> -               $(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
> +               $(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
>                 $(if $(V)$(DEBUG), --debug) \
>                 $(if $(DEBUG), --interactive) \
>                 $(if $(J),--jobs $(J)) \
> @@ -128,9 +136,9 @@ vm-boot-serial-%: $(IMAGES_DIR)/%.img
>                 -device virtio-net-pci,netdev=vnet \
>         || true
>
> -vm-boot-ssh-%: $(IMAGES_DIR)/%.img check-venv
> +vm-boot-ssh-%: $(IMAGES_DIR)/%.img $(VM_VENV)
>         $(call quiet-command, \
> -               $(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
> +               $(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
>                 $(if $(J),--jobs $(J)) \
>                 $(if $(V)$(DEBUG), --debug) \
>                 $(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
> --
> 2.36.1
>

as for this hack -- trusting your judgment on it for now, but I am not
sure I agree with your proposed long term fix just yet. I'll add it to
the list of criteria for the next series.

Acked-By: John Snow <jsnow@redhat.com>
diff mbox series

Patch

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 588bc999cc..5f5b1fbfe6 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -1,8 +1,17 @@ 
 # Makefile for VM tests
 
-.PHONY: vm-build-all vm-clean-all
+# Hack to allow running in an unconfigured build tree
+ifeq ($(wildcard $(SRC_PATH)/config-host.mak),)
+VM_PYTHON = PYTHONPATH=$(SRC_PATH)/python /usr/bin/env python3
+VM_VENV =
+HOST_ARCH := $(shell uname -m)
+else
+VM_PYTHON = $(TESTS_PYTHON)
+VM_VENV = check-venv
+HOST_ARCH = $(ARCH)
+endif
 
-HOST_ARCH = $(if $(ARCH),$(ARCH),$(shell uname -m))
+.PHONY: vm-build-all vm-clean-all
 
 EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
 
@@ -85,10 +94,10 @@  vm-clean-all:
 $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
 			$(SRC_PATH)/tests/vm/basevm.py \
 			$(SRC_PATH)/tests/vm/Makefile.include \
-			check-venv
+			$(VM_VENV)
 	@mkdir -p $(IMAGES_DIR)
 	$(call quiet-command, \
-		$(TESTS_PYTHON) $< \
+		$(VM_PYTHON) $< \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \
 		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \
@@ -100,11 +109,10 @@  $(IMAGES_DIR)/%.img:	$(SRC_PATH)/tests/vm/% \
 		--build-image $@, \
 		"  VM-IMAGE $*")
 
-
 # Build in VM $(IMAGE)
-vm-build-%: $(IMAGES_DIR)/%.img check-venv
+vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
 	$(call quiet-command, \
-		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
+		$(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(DEBUG), --interactive) \
 		$(if $(J),--jobs $(J)) \
@@ -128,9 +136,9 @@  vm-boot-serial-%: $(IMAGES_DIR)/%.img
 		-device virtio-net-pci,netdev=vnet \
 	|| true
 
-vm-boot-ssh-%: $(IMAGES_DIR)/%.img check-venv
+vm-boot-ssh-%: $(IMAGES_DIR)/%.img $(VM_VENV)
 	$(call quiet-command, \
-		$(TESTS_PYTHON) $(SRC_PATH)/tests/vm/$* \
+		$(VM_PYTHON) $(SRC_PATH)/tests/vm/$* \
 		$(if $(J),--jobs $(J)) \
 		$(if $(V)$(DEBUG), --debug) \
 		$(if $(QEMU_LOCAL),--build-path $(BUILD_DIR)) \