[RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block

John Snow posted 9 patches 3 years, 9 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block
Posted by John Snow 3 years, 9 months ago
This patch is being front-loaded before iotests actually relies on the
tests/venv being created in order to preserve bisectability.

Problems I am aware of here (There are a lot, sorry):

- I am not sure the right place to express this dependency, so I did it
  in tests/Makefile.include. It seems to work. I wasn't sure how to
  express it in tests/qemu-iotests/meson.build, but I am not sure if
  that would make it work for both "check" and "check-block" anyway.

- I don't really understand why I need empty rules for Make to process
  the pre-requisite. Without the "do-nothing" recipes, the venv building
  doesn't seem to trigger at all. What I have seems to work, but I'm
  worried I broke something unseen.

- This adds avocado as a dependency of "check"/"check-block", which is
  not technically true. It was just a sin of convenience to create one
  shared "testing venv". Maybe I'll figure out a scheme to have avocado's
  dependencies be "extras" that get added in to a standard "core set".

- This patch ignore the requisite that RPM builds be able to run without
  internet access, meaning that a PyPI fetch is not permissable. I plan
  to solve this by using an environment variable (QEMU_CHECK_NO_PYPI)
  that changes the behavior of the venv setup slightly, and qemu
  specfiles can opt-in to this behavior.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/Makefile.include | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index dfb678d379f..fa7af711fe5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -154,10 +154,17 @@ check-acceptance-deprecated-warning:
 
 check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
+# Before we delegate to meson, create the python venv for block tests.
+.PHONY: check-block
+check-block: check-venv
+	@echo  # Without some rule, this doesn't run at all. Why?
+
+
 # Consolidated targets
 
 .PHONY: check check-clean get-vm-images
-check:
+check: check-venv
+	@echo # ???
 
 check-build: run-ninja
 
-- 
2.34.1
Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block
Posted by Paolo Bonzini 3 years, 9 months ago
On 5/13/22 02:06, John Snow wrote:
>   meson, create the python venv for block tests.
> +.PHONY: check-block
> +check-block: check-venv
> +	@echo  # Without some rule, this doesn't run at all. Why?
> +
> +
>   # Consolidated targets
>   
>   .PHONY: check check-clean get-vm-images
> -check:
> +check: check-venv
> +	@echo # ???
>   

I think you need instead:

# The do-meson-check and do-meson-bench targets are defined in Makefile.mtest
do-meson-check do-meson-bench: check-venv

and I would even add "all" to the targets that create the virtual environment.

Paolo
Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block
Posted by John Snow 3 years, 9 months ago
On Fri, May 13, 2022, 4:41 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> >   meson, create the python venv for block tests.
> > +.PHONY: check-block
> > +check-block: check-venv
> > +     @echo  # Without some rule, this doesn't run at all. Why?
> > +
> > +
> >   # Consolidated targets
> >
> >   .PHONY: check check-clean get-vm-images
> > -check:
> > +check: check-venv
> > +     @echo # ???
> >
>
> I think you need instead:
>
> # The do-meson-check and do-meson-bench targets are defined in
> Makefile.mtest
> do-meson-check do-meson-bench: check-venv
>
> and I would even add "all" to the targets that create the virtual
> environment.
>
> Paolo
>

Great, thanks! I'll try that out today.
Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block
Posted by Paolo Bonzini 3 years, 9 months ago
On 5/13/22 16:12, John Snow wrote:
> 
>     I think you need instead:
> 
>     # The do-meson-check and do-meson-bench targets are defined in
>     Makefile.mtest
>     do-meson-check do-meson-bench: check-venv
> 
>     and I would even add "all" to the targets that create the virtual
>     environment.
> 
>     Paolo
> 
> 
> Great, thanks! I'll try that out today.

Well, check out the other suggestion of creating the venv at configure 
time, because that would remove all these complications/annoyances.

Paolo
Re: [RFC PATCH 6/9] tests: add check-venv as a dependency of check and check-block
Posted by John Snow 3 years, 9 months ago
On Fri, May 13, 2022, 11:34 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:12, John Snow wrote:
> >
> >     I think you need instead:
> >
> >     # The do-meson-check and do-meson-bench targets are defined in
> >     Makefile.mtest
> >     do-meson-check do-meson-bench: check-venv
> >
> >     and I would even add "all" to the targets that create the virtual
> >     environment.
> >
> >     Paolo
> >
> >
> > Great, thanks! I'll try that out today.
>
> Well, check out the other suggestion of creating the venv at configure
> time, because that would remove all these complications/annoyances.
>
> Paolo
>

They also raise new annoyances and questions for me, so it might be worth
updating this "branch" of the patchset to have a basis of comparison for
what's the least annoying in the end.

(Or maybe even to serve as a basis while transitioning to the "better"
solution. It's quick to try, at least.)

Config script ideas are gonna take me a bit longer to work through.

(I'm not against developing a minimum viable patchset and having you tweak
it to your desire afterwards, if you have the time/interest. We can chat on
irc if you'd like. Otherwise, I'll just push forward.)

--js

>