diff mbox series

[v4,1/3] Bootstrap Python venv for tests

Message ID 20181012165347.2117-2-crosa@redhat.com
State New
Headers show
Series Bootstrap Python venv and acceptance/functional tests | expand

Commit Message

Cleber Rosa Oct. 12, 2018, 4:53 p.m. UTC
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.

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

Comments

Philippe Mathieu-Daudé Oct. 12, 2018, 9:30 p.m. UTC | #1
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

> 
> 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"

> +
> +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 Habkost Oct. 13, 2018, 3:37 a.m. UTC | #2
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>
Caio Carrara Oct. 15, 2018, 6:41 p.m. UTC | #3
On 13-10-2018 00:37, Eduardo Habkost wrote:
> 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.
> 

Actually this is not necessary since the avocado is being called from
the "venv python binary" as you can see in the check-acceptance target.

This way all the requirements installed in the test venv can be used
without activating the virtual environment.

> 
>>
>>> +
>>> +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>
>
Caio Carrara Oct. 15, 2018, 7:04 p.m. UTC | #4
On 12-10-2018 13: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.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/Makefile.include      | 20 ++++++++++++++++++++
>  tests/venv-requirements.txt |  3 +++

Any special reason to name `venv-requirements.txt` instead of only
`requirements.txt`? I think the second way is better (if there's no
cons) since it's the default from most of Python projects. Besides that
seems more semantic to have a `tests/requirements.txt` file that
registers the requirements for tests.

Still in this topic. As far as I could see, the files in `tests/`
directory are almost all C source code. The right place for the Python
requirements seems the `tests/acceptance/` directory specially because
the subject of all this patches. Keeping only one requirements file for
all kinds of tests (that uses Python) can be problematic in my opinion.
If it makes any sense, probably is also a good idea rename the target
from `check-venv` to something like `check-acceptance-venv`.

>  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 $@)
> +
> +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
>
Philippe Mathieu-Daudé Oct. 15, 2018, 10:22 p.m. UTC | #5
On 15/10/2018 21:04, Caio Carrara wrote:
> On 12-10-2018 13: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.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/Makefile.include      | 20 ++++++++++++++++++++
>>  tests/venv-requirements.txt |  3 +++
> 
> Any special reason to name `venv-requirements.txt` instead of only
> `requirements.txt`? I think the second way is better (if there's no
> cons) since it's the default from most of Python projects. Besides that
> seems more semantic to have a `tests/requirements.txt` file that
> registers the requirements for tests.

Agreed.

> 
> Still in this topic. As far as I could see, the files in `tests/`
> directory are almost all C source code. The right place for the Python
> requirements seems the `tests/acceptance/` directory specially because
> the subject of all this patches. Keeping only one requirements file for
> all kinds of tests (that uses Python) can be problematic in my opinion.
> If it makes any sense, probably is also a good idea rename the target
> from `check-venv` to something like `check-acceptance-venv`.

Yes.

> 
>>  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 $@)
>> +
>> +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
>>
> 
>
Philippe Mathieu-Daudé Oct. 15, 2018, 10:28 p.m. UTC | #6
Hi Caio,

On 15/10/2018 20:41, Caio Carrara wrote:
> On 13-10-2018 00:37, Eduardo Habkost wrote:
>> 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.
>>
> 
> Actually this is not necessary since the avocado is being called from
> the "venv python binary" as you can see in the check-acceptance target.
> 
> This way all the requirements installed in the test venv can be used
> without activating the virtual environment.

Well this is only true if you call 'make check-acceptance', not if you
want to filter tests, or run the single file you are working on...
Or am I missing something? The only option user-configurable (without
activating the venv) is the output of all tests via the AVOCADO_SHOW env
var.

This might be enough for a maintainer checking his subsystem, but I
don't find this practical for a acceptance test writer. And we want for
people to contribute adding tests, right?
Well, if we have maintainer running them, this is already a win :)

> 
>>
>>>
>>>> +
>>>> +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 Habkost Oct. 15, 2018, 10:40 p.m. UTC | #7
On Tue, Oct 16, 2018 at 12:28:07AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Caio,
> 
> On 15/10/2018 20:41, Caio Carrara wrote:
> > On 13-10-2018 00:37, Eduardo Habkost wrote:
> >> 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.
> >>
> > 
> > Actually this is not necessary since the avocado is being called from
> > the "venv python binary" as you can see in the check-acceptance target.
> > 
> > This way all the requirements installed in the test venv can be used
> > without activating the virtual environment.
> 
> Well this is only true if you call 'make check-acceptance', not if you
> want to filter tests, or run the single file you are working on...
> Or am I missing something? The only option user-configurable (without
> activating the venv) is the output of all tests via the AVOCADO_SHOW env
> var.
> 
> This might be enough for a maintainer checking his subsystem, but I
> don't find this practical for a acceptance test writer. And we want for
> people to contribute adding tests, right?
> Well, if we have maintainer running them, this is already a win :)
> 

Good point: these are important use cases too.

Now, we need to decide what's the best interface for performing
those tasks.

Existing unit tests and qtest-based tests use Makefile variables
to select test cases to run.  But I'm not sure this is the most
usable way to do it.

Telling people to manually activate the venv and run avocado
manually doesn't sound desirable to me: people would get a
completely different behavior from `check-acceptance`: they'll
get log files in a different location, and get confused if extra
avocado arguments are required to make some tests work.

Personally, I think most people would be more comfortable using a
simple `./tests/acceptance/run` wrapper script, that would
transparently invoke avocado inside the venv with the right
arguments.

Bonus points if we make it possible to execute single test cases
directly using `python tests/acceptance/mytestcase.py` or
`./tests/acceptance/mytestcase.py`.


> > 
> >>
> >>>
> >>>> +
> >>>> +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>
> >>
> > 
> >
Cleber Rosa Oct. 16, 2018, 1:50 p.m. UTC | #8
On 10/12/18 11:37 PM, Eduardo Habkost wrote:
> 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.
> 
> 

IIUC, printing a "Can't create tests venv on Python 2", and aborting,
would be enough, right?

- Cleber.

>>
>>>
>>> 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>
>
Cleber Rosa Oct. 16, 2018, 1:51 p.m. UTC | #9
On 10/15/18 2:41 PM, Caio Carrara wrote:
> On 13-10-2018 00:37, Eduardo Habkost wrote:
>> 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.
>>
> 
> Actually this is not necessary since the avocado is being called from
> the "venv python binary" as you can see in the check-acceptance target.
> 

Exactly.

> This way all the requirements installed in the test venv can be used
> without activating the virtual environment.
> 

Exactly.

- Cleber.

>>
>>>
>>>> +
>>>> +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>
>>
> 
>
Cleber Rosa Oct. 16, 2018, 1:56 p.m. UTC | #10
On 10/15/18 6:28 PM, Philippe Mathieu-Daudé wrote:
> Hi Caio,
> 
> On 15/10/2018 20:41, Caio Carrara wrote:
>> On 13-10-2018 00:37, Eduardo Habkost wrote:
>>> 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.
>>>
>>
>> Actually this is not necessary since the avocado is being called from
>> the "venv python binary" as you can see in the check-acceptance target.
>>
>> This way all the requirements installed in the test venv can be used
>> without activating the virtual environment.
> 
> Well this is only true if you call 'make check-acceptance', not if you
> want to filter tests, or run the single file you are working on...
> Or am I missing something? The only option user-configurable (without
> activating the venv) is the output of all tests via the AVOCADO_SHOW env
> var.

Allowing someone to just run `make check-acceptance` is the primary and
most important goal here IMO.  It's supposed to be simple, which means,
not having too many knobs.

> 
> This might be enough for a maintainer checking his subsystem, but I
> don't find this practical for a acceptance test writer. And we want for
> people to contribute adding tests, right?
> Well, if we have maintainer running them, this is already a win :)
> 

Today, with the code AS-IS, you can still do:

 $ tests/venv/bin/avocado run [all your options] tests/acceptance

I think the subsystem and maintainers question will come, but it's a
different discussion.  Contradicting myself now and getting into that
discussion, :P, it could be as simple as `make TAG=foo check-acceptance`
that gets passed to Avocado.  Or, in line with other check targets,
`make check-acceptance-foo`.

- Cleber.

>>
>>>
>>>>
>>>>> +
>>>>> +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>
>>>
>>
>>
Cleber Rosa Oct. 16, 2018, 2:08 p.m. UTC | #11
On 10/15/18 6:40 PM, Eduardo Habkost wrote:
> On Tue, Oct 16, 2018 at 12:28:07AM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Caio,
>>
>> On 15/10/2018 20:41, Caio Carrara wrote:
>>> On 13-10-2018 00:37, Eduardo Habkost wrote:
>>>> 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.
>>>>
>>>
>>> Actually this is not necessary since the avocado is being called from
>>> the "venv python binary" as you can see in the check-acceptance target.
>>>
>>> This way all the requirements installed in the test venv can be used
>>> without activating the virtual environment.
>>
>> Well this is only true if you call 'make check-acceptance', not if you
>> want to filter tests, or run the single file you are working on...
>> Or am I missing something? The only option user-configurable (without
>> activating the venv) is the output of all tests via the AVOCADO_SHOW env
>> var.
>>
>> This might be enough for a maintainer checking his subsystem, but I
>> don't find this practical for a acceptance test writer. And we want for
>> people to contribute adding tests, right?
>> Well, if we have maintainer running them, this is already a win :)
>>
> 
> Good point: these are important use cases too.
> 
> Now, we need to decide what's the best interface for performing
> those tasks.
> 
> Existing unit tests and qtest-based tests use Makefile variables
> to select test cases to run.  But I'm not sure this is the most
> usable way to do it.
> 

I also fear about getting too deep into the Makefiles, adding content
for every new test, etc.  It's certainly not the way to go here.

> Telling people to manually activate the venv and run avocado
> manually doesn't sound desirable to me: people would get a
> completely different behavior from `check-acceptance`: they'll
> get log files in a different location, and get confused if extra
> avocado arguments are required to make some tests work.
> 

Agreed.  The whole point of this work, IMO, is to provide a seamless and
transparent way to execute the most common task.  At this point, we
should be telling people to run *all* tests we have, so selecting
specific tests is something that we have some time to deal with.

> Personally, I think most people would be more comfortable using a
> simple `./tests/acceptance/run` wrapper script, that would
> transparently invoke avocado inside the venv with the right
> arguments.
> 

I have something in mind which seems to relate to your idea of `run`.
Basically, when we get to the point of having more complex test suites,
we can have "avocado job scripts", using the "Job API", to create and
run jobs with a specific selection of tests and custom options (such as
specific varianters for some tests, etc).

For instance, we may want to have a "job_storage_migration.py", an
Avocado job script (not a test), that includes a pre-tests plugin
execution that sets up some storage, a test suitewith a few acceptance
tests, another test suite with some iotests run with different variants,
and a post-tests plugin that cleans up the environment.

Until then, I don't know what I would put into `run`.  A command that
calls `make check-acceptance`?  I'm confused by that.

> Bonus points if we make it possible to execute single test cases
> directly using `python tests/acceptance/mytestcase.py` or
> `./tests/acceptance/mytestcase.py`.
> 

This is possible with:

#!/usr/bin/env python

from avocado import main

[test]

if __name__ == "__main__":
    main()

But is it really worth it?  IMO, it's not.

- Cleber.

> 
>>>
>>>>
>>>>>
>>>>>> +
>>>>>> +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>
>>>>
>>>
>>>
>
Cleber Rosa Oct. 16, 2018, 2:17 p.m. UTC | #12
On 10/15/18 3:04 PM, Caio Carrara wrote:
> On 12-10-2018 13: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.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/Makefile.include      | 20 ++++++++++++++++++++
>>  tests/venv-requirements.txt |  3 +++
> 
> Any special reason to name `venv-requirements.txt` instead of only
> `requirements.txt`? I think the second way is better (if there's no
> cons) since it's the default from most of Python projects. Besides that
> seems more semantic to have a `tests/requirements.txt` file that
> registers the requirements for tests.
> 

It was a decision based on the fact that other components may have other
requirements, basically a namespace thing and a "explicit is better than
implicit" thing.

The env-requirements.txt file is not for "tests" in general, but for the
tests that we want to be run out of the venv.  This can quickly become a
different thing (look at the tests/docker, tests/vm, tests/migration, etc).

> Still in this topic. As far as I could see, the files in `tests/`
> directory are almost all C source code. The right place for the Python
> requirements seems the `tests/acceptance/` directory specially because
> the subject of all this patches. Keeping only one requirements file for
> all kinds of tests (that uses Python) can be problematic in my opinion.
> If it makes any sense, probably is also a good idea rename the target
> from `check-venv` to something like `check-acceptance-venv`.
> 

I agree with some of your points here.  The way I see this working is
either:

1) Have a single venv created for running Python based tests, and have
its requirements listed in tests/venv-requirements.txt

2) Have one venv created for every type of Python based tests, and have
requirements.txt in the specific directories (such as
tests/acceptance/requirements.txt, tests/vm/requirements.txt), and
multiple `check-[type]-venv` targets.

I obviously chose 1, simply because I find it overkill to have different
venvs at this point.  I don't see the additional value option #2 would
have at this point.

- Cleber.

>>  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 $@)
>> +
>> +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
>>
> 
>
Philippe Mathieu-Daudé Oct. 16, 2018, 2:20 p.m. UTC | #13
Le mar. 16 oct. 2018 16:08, Cleber Rosa <crosa@redhat.com> a écrit :

>
>
> On 10/15/18 6:40 PM, Eduardo Habkost wrote:
> > On Tue, Oct 16, 2018 at 12:28:07AM +0200, Philippe Mathieu-Daudé wrote:
> >> Hi Caio,
> >>
> >> On 15/10/2018 20:41, Caio Carrara wrote:
> >>> On 13-10-2018 00:37, Eduardo Habkost wrote:
> >>>> 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.
> >>>>
> >>>
> >>> Actually this is not necessary since the avocado is being called from
> >>> the "venv python binary" as you can see in the check-acceptance target.
> >>>
> >>> This way all the requirements installed in the test venv can be used
> >>> without activating the virtual environment.
> >>
> >> Well this is only true if you call 'make check-acceptance', not if you
> >> want to filter tests, or run the single file you are working on...
> >> Or am I missing something? The only option user-configurable (without
> >> activating the venv) is the output of all tests via the AVOCADO_SHOW env
> >> var.
> >>
> >> This might be enough for a maintainer checking his subsystem, but I
> >> don't find this practical for a acceptance test writer. And we want for
> >> people to contribute adding tests, right?
> >> Well, if we have maintainer running them, this is already a win :)
> >>
> >
> > Good point: these are important use cases too.
> >
> > Now, we need to decide what's the best interface for performing
> > those tasks.
> >
> > Existing unit tests and qtest-based tests use Makefile variables
> > to select test cases to run.  But I'm not sure this is the most
> > usable way to do it.
> >
>
> I also fear about getting too deep into the Makefiles, adding content
> for every new test, etc.  It's certainly not the way to go here.
>
> > Telling people to manually activate the venv and run avocado
> > manually doesn't sound desirable to me: people would get a
> > completely different behavior from `check-acceptance`: they'll
> > get log files in a different location, and get confused if extra
> > avocado arguments are required to make some tests work.
> >
>
> Agreed.  The whole point of this work, IMO, is to provide a seamless and
> transparent way to execute the most common task.  At this point, we
> should be telling people to run *all* tests we have, so selecting
> specific tests is something that we have some time to deal with.
>
> > Personally, I think most people would be more comfortable using a
> > simple `./tests/acceptance/run` wrapper script, that would
> > transparently invoke avocado inside the venv with the right
> > arguments.
> >
>
> I have something in mind which seems to relate to your idea of `run`.
> Basically, when we get to the point of having more complex test suites,
> we can have "avocado job scripts", using the "Job API", to create and
> run jobs with a specific selection of tests and custom options (such as
> specific varianters for some tests, etc).
>
> For instance, we may want to have a "job_storage_migration.py", an
> Avocado job script (not a test), that includes a pre-tests plugin
> execution that sets up some storage, a test suitewith a few acceptance
> tests, another test suite with some iotests run with different variants,
> and a post-tests plugin that cleans up the environment.
>
> Until then, I don't know what I would put into `run`.  A command that
> calls `make check-acceptance`?  I'm confused by that.
>
> > Bonus points if we make it possible to execute single test cases
> > directly using `python tests/acceptance/mytestcase.py` or
> > `./tests/acceptance/mytestcase.py`.
> >
>
> This is possible with:
>
> #!/usr/bin/env python
>
> from avocado import main
>
> [test]
>
> if __name__ == "__main__":
>     main()


> But is it really worth it?  IMO, it's not.
>

If that seamlessly uses the venv, I think it is.


> - Cleber.
>
> >
> >>>
> >>>>
> >>>>>
> >>>>>> +
> >>>>>> +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>
> >>>>
> >>>
> >>>
> >
>
> --
> Cleber Rosa
> [ Sr Software Engineer - Virtualization Team - Red Hat ]
> [ Avocado Test Framework - avocado-framework.github.io ]
> [  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]
>
Cleber Rosa Oct. 16, 2018, 2:22 p.m. UTC | #14
On 10/15/18 6:22 PM, Philippe Mathieu-Daudé wrote:
> On 15/10/2018 21:04, Caio Carrara wrote:
>> On 12-10-2018 13: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.
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>  tests/Makefile.include      | 20 ++++++++++++++++++++
>>>  tests/venv-requirements.txt |  3 +++
>>
>> Any special reason to name `venv-requirements.txt` instead of only
>> `requirements.txt`? I think the second way is better (if there's no
>> cons) since it's the default from most of Python projects. Besides that
>> seems more semantic to have a `tests/requirements.txt` file that
>> registers the requirements for tests.
> 
> Agreed.
> 

I found it to be more explicit.  Naming it "requirements.txt", is
usually done in first-level directories of Python projects.  So, I'd put
it in "tests/acceptance/requirements.txt" if it was supposed to be used
*only* by acceptance tests (see previous response).

But, if the majority prefers the more traditional name, I can certainly
go with it.

>>
>> Still in this topic. As far as I could see, the files in `tests/`
>> directory are almost all C source code. The right place for the Python
>> requirements seems the `tests/acceptance/` directory specially because
>> the subject of all this patches. Keeping only one requirements file for
>> all kinds of tests (that uses Python) can be problematic in my opinion.
>> If it makes any sense, probably is also a good idea rename the target
>> from `check-venv` to something like `check-acceptance-venv`.
> 
> Yes.
> 

I disagree because that was not my original motivation.  As explained in
the previous response, the motivation was to have a venv that could be
used for running all Python based tests.

- Cleber.

>>
>>>  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 $@)
>>> +
>>> +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
>>>
>>
>>
Cleber Rosa Oct. 16, 2018, 2:44 p.m. UTC | #15
On 10/16/18 10:20 AM, Philippe Mathieu-Daudé wrote:

>>
>> This is possible with:
>>
>> #!/usr/bin/env python
>>
>> from avocado import main
>>
>> [test]
>>
>> if __name__ == "__main__":
>>     main()
> 
> 
>> But is it really worth it?  IMO, it's not.
>>
> 
> If that seamlessly uses the venv, I think it is.
> 
>

I would not seamlessly use the venv, unless the venv is activated.

To me, we're clearly trying to stretch a very simple thing (the venv)
into something that should automagically do a lot more, but can't easily
do so.

- Cleber.
Philippe Mathieu-Daudé Oct. 16, 2018, 2:58 p.m. UTC | #16
On 16/10/2018 15:50, Cleber Rosa wrote:

> On 10/12/18 11:37 PM, Eduardo Habkost wrote:
>> 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.
>>
>>
> 
> IIUC, printing a "Can't create tests venv on Python 2", and aborting,
> would be enough, right?

Certainly.

> 
> - Cleber.
> 
>>>
>>>>
>>>> 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>
>>
>
Philippe Mathieu-Daudé Oct. 16, 2018, 3:04 p.m. UTC | #17
On 16/10/2018 15:56, Cleber Rosa wrote:
> On 10/15/18 6:28 PM, Philippe Mathieu-Daudé wrote:
>> Hi Caio,
>>
>> On 15/10/2018 20:41, Caio Carrara wrote:
>>> On 13-10-2018 00:37, Eduardo Habkost wrote:
>>>> 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.
>>>>
>>>
>>> Actually this is not necessary since the avocado is being called from
>>> the "venv python binary" as you can see in the check-acceptance target.
>>>
>>> This way all the requirements installed in the test venv can be used
>>> without activating the virtual environment.
>>
>> Well this is only true if you call 'make check-acceptance', not if you
>> want to filter tests, or run the single file you are working on...
>> Or am I missing something? The only option user-configurable (without
>> activating the venv) is the output of all tests via the AVOCADO_SHOW env
>> var.
> 
> Allowing someone to just run `make check-acceptance` is the primary and
> most important goal here IMO.  It's supposed to be simple, which means,
> not having too many knobs.
> 
>>
>> This might be enough for a maintainer checking his subsystem, but I
>> don't find this practical for a acceptance test writer. And we want for
>> people to contribute adding tests, right?
>> Well, if we have maintainer running them, this is already a win :)
>>
> 
> Today, with the code AS-IS, you can still do:
> 
>  $ tests/venv/bin/avocado run [all your options] tests/acceptance

If this is documented, we don't need the other tricks and your series is
fine as it!

> 
> I think the subsystem and maintainers question will come, but it's a
> different discussion.  Contradicting myself now and getting into that
> discussion, :P, it could be as simple as `make TAG=foo check-acceptance`
> that gets passed to Avocado.  Or, in line with other check targets,
> `make check-acceptance-foo`.
> 
> - Cleber.
> 
>>>
>>>>
>>>>>
>>>>>> +
>>>>>> +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>
>>>>
>>>
>>>
>
diff mbox series

Patch

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 $@)
+
+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