diff mbox series

[1/2] Bootstrap Python venv for tests

Message ID 20180920151957.28803-2-crosa@redhat.com
State New
Headers show
Series [1/2] Bootstrap Python venv for tests | expand

Commit Message

Cleber Rosa Sept. 20, 2018, 3:19 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 requiment, 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      | 19 +++++++++++++++++++
 tests/venv-requirements.txt |  3 +++
 2 files changed, 22 insertions(+)
 create mode 100644 tests/venv-requirements.txt

Comments

Philippe Mathieu-Daudé Sept. 20, 2018, 11:29 p.m. UTC | #1 
Hi Cleber,

On 9/20/18 5:19 PM, 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 requiment, 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.

Very good idea!

> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/Makefile.include      | 19 +++++++++++++++++++
>  tests/venv-requirements.txt |  3 +++
>  2 files changed, 22 insertions(+)
>  create mode 100644 tests/venv-requirements.txt
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 87c81d1dcc..9bb90a83d4 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"
> @@ -999,6 +1000,23 @@ 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=$(BUILD_DIR)/tests/venv-requirements.txt

$ make check-venv
  VENV    build/tests/venv
  PIP     build/tests/venv-requirements.txt
Could not open requirements file: [Errno 2] No such file or directory:
'build/tests/venv-requirements.txt'
make: *** [tests/Makefile.include:1012: build/tests/venv] Error 1

This should be:

TESTS_VENV_REQ=$(SRC_PATH)/tests/venv-requirements.txt

> +
> +$(TESTS_VENV_DIR):
> +	$(call quiet-command, \
> +            $(PYTHON) -m venv $@, \
> +            VENV, $@)
> +	$(call quiet-command, \
> +            $(TESTS_VENV_DIR)/bin/pip -q install -r $(TESTS_VENV_REQ), \
> +            PIP, $(TESTS_VENV_REQ))
> +
> +check-venv: $(TESTS_VENV_DIR)

I'm not sure testing the directory creation is enough:

$ make check-venv
  VENV    build/tests/venv
The virtual environment was not created successfully because ensurepip
is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt-get install python3-venv

You may need to use sudo with that command.  After installing the
python3-venv
package, recreate your virtual environment.

Failing command: ['build/tests/venv/bin/python3', '-Im', 'ensurepip',
'--upgrade', '--default-pip']

make: *** [tests/Makefile.include:1011: build/tests/venv] Error 1

$ make check-venv
make: Nothing to be done for 'check-venv'.

The dir is here but the venv is incomplete.


Also, once applied your patch 2 which add the avocado dependency,
running 'check-env' doesn't install the new requirements:

$ make check-venv
make: Nothing to be done for 'check-venv'.

Maybe a make rule checking $(TESTS_VENV_REQ) is enough to trigger an
install.

> +
>  # Consolidated targets
>  
>  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
> @@ -1012,6 +1030,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
> 

Regards,

Phil.
Cleber Rosa Sept. 21, 2018, 6:30 p.m. UTC | #2 
On 9/20/18 7:29 PM, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 9/20/18 5:19 PM, 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 requiment, 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.
> 
> Very good idea!
> 
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/Makefile.include      | 19 +++++++++++++++++++
>>  tests/venv-requirements.txt |  3 +++
>>  2 files changed, 22 insertions(+)
>>  create mode 100644 tests/venv-requirements.txt
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 87c81d1dcc..9bb90a83d4 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"
>> @@ -999,6 +1000,23 @@ 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=$(BUILD_DIR)/tests/venv-requirements.txt
> 
> $ make check-venv
>   VENV    build/tests/venv
>   PIP     build/tests/venv-requirements.txt
> Could not open requirements file: [Errno 2] No such file or directory:
> 'build/tests/venv-requirements.txt'
> make: *** [tests/Makefile.include:1012: build/tests/venv] Error 1
> 
> This should be:
> 
> TESTS_VENV_REQ=$(SRC_PATH)/tests/venv-requirements.txt
> 

Right, nice catch.

>> +
>> +$(TESTS_VENV_DIR):
>> +	$(call quiet-command, \
>> +            $(PYTHON) -m venv $@, \
>> +            VENV, $@)
>> +	$(call quiet-command, \
>> +            $(TESTS_VENV_DIR)/bin/pip -q install -r $(TESTS_VENV_REQ), \
>> +            PIP, $(TESTS_VENV_REQ))
>> +
>> +check-venv: $(TESTS_VENV_DIR)
> 
> I'm not sure testing the directory creation is enough:
> 

I see your point.  On a previous version, I was looking at specific
Python files ("<module-name>.dist-info") as an extra condition for
successful module installation, but it was too much Python and module
version specific.  Maybe we can add a final command that touches a file
as a flag, say "touch $(TESTS_VENV_DIR)/.install_complete".

> $ make check-venv
>   VENV    build/tests/venv
> The virtual environment was not created successfully because ensurepip
> is not
> available.  On Debian/Ubuntu systems, you need to install the python3-venv
> package using the following command.
> 
>     apt-get install python3-venv
> 
> You may need to use sudo with that command.  After installing the
> python3-venv
> package, recreate your virtual environment.
> 

On Fedora/EL systems, python3 requires both python3-libs (which contains
venv) and python3-pip.  Thanks for testing on Debian/Ubuntu.

Now that questions is whether "tests-venv" should try to do anything
system wide or that requires sudo: I personally don't think it should.

> Failing command: ['build/tests/venv/bin/python3', '-Im', 'ensurepip',
> '--upgrade', '--default-pip']
> 
> make: *** [tests/Makefile.include:1011: build/tests/venv] Error 1
> 
> $ make check-venv
> make: Nothing to be done for 'check-venv'.
> 
> The dir is here but the venv is incomplete.
> 
> 
> Also, once applied your patch 2 which add the avocado dependency,
> running 'check-env' doesn't install the new requirements:
> 
> $ make check-venv
> make: Nothing to be done for 'check-venv'.
> 
> Maybe a make rule checking $(TESTS_VENV_REQ) is enough to trigger an
> install.
> 

That was also a simplification done purpose after the experiments with
the "<module-name>.dist-info" checks I mentioned before.  But, I think
we can fix that by having pip generate a timestamp flag file.  So, if
"venv-requirements.txt" changes, pip will execute again.

Thanks for the suggestions and testing here!
- Cleber.

>> +
>>  # Consolidated targets
>>  
>>  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
>> @@ -1012,6 +1030,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
>>
> 
> Regards,
> 
> Phil.
>
Philippe Mathieu-Daudé Sept. 27, 2018, 10:49 a.m. UTC | #3 
Hi Cleber,

On 20/09/2018 16:19, 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 requiment, 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      | 19 +++++++++++++++++++
>  tests/venv-requirements.txt |  3 +++
>  2 files changed, 22 insertions(+)
>  create mode 100644 tests/venv-requirements.txt
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 87c81d1dcc..9bb90a83d4 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"
> @@ -999,6 +1000,23 @@ 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=$(BUILD_DIR)/tests/venv-requirements.txt
> +
> +$(TESTS_VENV_DIR):
> +	$(call quiet-command, \
> +            $(PYTHON) -m venv $@, \

Following your suggestion to test using Python2 from
https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg03537.html

I'm getting:

/usr/bin/python2: No module named venv

So we'd have to use 'python2 -m virtualenv' here.

Anyway I'd prefer to not use/support python2 for acceptance testing.

> +            VENV, $@)
> +	$(call quiet-command, \
> +            $(TESTS_VENV_DIR)/bin/pip -q install -r $(TESTS_VENV_REQ), \
> +            PIP, $(TESTS_VENV_REQ))
> +
> +check-venv: $(TESTS_VENV_DIR)
> +
>  # Consolidated targets
>  
>  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
> @@ -1012,6 +1030,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
>
diff mbox series

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 87c81d1dcc..9bb90a83d4 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"
@@ -999,6 +1000,23 @@  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=$(BUILD_DIR)/tests/venv-requirements.txt
+
+$(TESTS_VENV_DIR):
+	$(call quiet-command, \
+            $(PYTHON) -m venv $@, \
+            VENV, $@)
+	$(call quiet-command, \
+            $(TESTS_VENV_DIR)/bin/pip -q install -r $(TESTS_VENV_REQ), \
+            PIP, $(TESTS_VENV_REQ))
+
+check-venv: $(TESTS_VENV_DIR)
+
 # Consolidated targets
 
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
@@ -1012,6 +1030,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