Re: [Qemu-devel] [PATCH v4 1/3] Bootstrap Python venv for tests

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Oct 12, 2018 at 11:30:39PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 12/10/2018 18:53, Cleber Rosa wrote:
> > A number of QEMU tests are written in Python, and may benefit
> > from an untainted Python venv.
> > 
> > By using make rules, tests that depend on specific Python libs
> > can set that rule as a requirement, along with rules that require
> > the presence or installation of specific libraries.
> > 
> > The tests/venv-requirements.txt is supposed to contain the
> > Python requirements that should be added to the venv created
> > by check-venv.
> 
> Maybe you (or Eduardo...) what you wrote in the cover:
> 
>  There's one current caveat: it requires Python 3, as it's based on the
>  venv module.
> 
> To explain:
> 
> $ make check-acceptance
> /usr/bin/python2: No module named venv
> make: *** [/home/phil/source/qemu/tests/Makefile.include:1033:] Error 1

Oops, this doesn't look very friendly.

But note that this would become a non-issue if we start requiring
Python 3 for building QEMU.


> 
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  tests/Makefile.include      | 20 ++++++++++++++++++++
> >  tests/venv-requirements.txt |  3 +++
> >  2 files changed, 23 insertions(+)
> >  create mode 100644 tests/venv-requirements.txt
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 5eadfd52f9..b66180efa1 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -12,6 +12,7 @@ check-help:
> >  	@echo " $(MAKE) check-block          Run block tests"
> >  	@echo " $(MAKE) check-tcg            Run TCG tests"
> >  	@echo " $(MAKE) check-report.html    Generates an HTML test report"
> > +	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
> >  	@echo " $(MAKE) check-clean          Clean the tests"
> >  	@echo
> >  	@echo "Please note that HTML reports do not regenerate if the unit tests"
> > @@ -1017,6 +1018,24 @@ check-decodetree:
> >            ./check.sh "$(PYTHON)" "$(SRC_PATH)/scripts/decodetree.py", \
> >            TEST, decodetree.py)
> >  
> > +# Python venv for running tests
> > +
> > +.PHONY: check-venv
> > +
> > +TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
> > +TESTS_VENV_REQ=$(SRC_PATH)/tests/venv-requirements.txt
> > +
> > +$(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> > +	$(call quiet-command, \
> > +            $(PYTHON) -m venv --system-site-packages $@, \
> > +            VENV, $@)
> > +	$(call quiet-command, \
> > +            $(TESTS_VENV_DIR)/bin/python -m pip -q install -r $(TESTS_VENV_REQ), \
> > +            PIP, $(TESTS_VENV_REQ))
> > +	$(call quiet-command, touch $@)
> 
> Hmm maybe we should print something like:
> 
>   "You can now activate this virtual environment using:
>     source $(TESTS_VENV_DIR)/tests/venv/bin/activate"

I'm not sure this would be necessary: I expect usage of the venv
to be completely transparent.

If we require people to learn what venv is and manually activate
it, I'd say we have failed to provide usable tools for running
the tests.


> 
> > +
> > +check-venv: $(TESTS_VENV_DIR)
> > +
> >  # Consolidated targets
> >  
> >  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
> > @@ -1030,6 +1049,7 @@ check-clean:
> >  	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
> >  	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
> >  	rm -f tests/test-qapi-gen-timestamp
> > +	rm -rf $(TESTS_VENV_DIR)
> >  
> >  clean: check-clean
> >  
> > diff --git a/tests/venv-requirements.txt b/tests/venv-requirements.txt
> > new file mode 100644
> > index 0000000000..d39f9d1576
> > --- /dev/null
> > +++ b/tests/venv-requirements.txt
> > @@ -0,0 +1,3 @@
> > +# Add Python module requirements, one per line, to be installed
> > +# in the tests/venv Python virtual environment. For more info,
> > +# refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> > 
> 
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

-- 
Eduardo