diff mbox series

[08/11] python: add 'make check-venv' invocation

Message ID 20210625154540.783306-9-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series Python: packaging cleanups | expand

Commit Message

John Snow June 25, 2021, 3:45 p.m. UTC
This is a *third* way to run the Python tests. Unlike the first two
(check-pipenv, check-tox), this version does not require any specific
interpreter version -- making it a lot easier to tell people to run it
as a quick smoketest prior to submission to GitLab CI.

Summary:

  Checked via GitLab CI:
    - check-pipenv: tests our oldest python & dependencies
    - check-tox: tests newest dependencies on all non-EOL python versions
  Executed only incidentally:
    - check-venv: tests newest dependencies on whichever python version

('make check' does not set up any environment at all, it just runs the
tests in your current environment. All four invocations perform the
exact same tests, just in different execution environments.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Makefile | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

Comments

Willian Rampazzo June 25, 2021, 6:36 p.m. UTC | #1
On Fri, Jun 25, 2021 at 12:46 PM John Snow <jsnow@redhat.com> wrote:
>
> This is a *third* way to run the Python tests. Unlike the first two
> (check-pipenv, check-tox), this version does not require any specific
> interpreter version -- making it a lot easier to tell people to run it
> as a quick smoketest prior to submission to GitLab CI.
>
> Summary:
>
>   Checked via GitLab CI:
>     - check-pipenv: tests our oldest python & dependencies
>     - check-tox: tests newest dependencies on all non-EOL python versions
>   Executed only incidentally:
>     - check-venv: tests newest dependencies on whichever python version
>
> ('make check' does not set up any environment at all, it just runs the
> tests in your current environment. All four invocations perform the
> exact same tests, just in different execution environments.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/Makefile | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 5cb8378b81..76bb24e671 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -1,4 +1,6 @@
> -.PHONY: help pipenv check-pipenv check clean distclean develop
> +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
> +
> +QEMU_VENV_DIR=~/.cache/qemu-pyvenv
>
>  help:
>         @echo "python packaging help:"
> @@ -15,6 +17,11 @@ help:
>         @echo "    Requires: Python 3.6-3.10 and tox."
>         @echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
>         @echo ""
> +       @echo "make check-venv:"

Maybe, it may confuse people using `make check-venv` under `tests`.
Anyway, I'm not opposed to it.

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

> +       @echo "    Run tests in a venv against your default python3 version."
> +       @echo "    These tests use the newest dependencies."
> +       @echo "    Requires: Python 3.x"
> +       @echo ""
>         @echo "make develop:    Install deps for 'make check', and"
>         @echo "                 the qemu libs in editable/development mode."
>         @echo ""
> @@ -23,6 +30,9 @@ help:
>         @echo "make pipenv"
>         @echo "    Creates pipenv's virtual environment (.venv)"
>         @echo ""
> +       @echo "make venv"
> +       @echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
> +       @echo ""
>         @echo "make clean:      remove package build output."
>         @echo ""
>         @echo "make distclean:  remove venv files, qemu package forwarder,"
> @@ -37,8 +47,27 @@ pipenv: .venv
>  check-pipenv: pipenv
>         @pipenv run make check
>
> +venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
> +$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
> +       @echo "VENV $(QEMU_VENV_DIR)"
> +       @python3 -m venv $(QEMU_VENV_DIR)
> +       @(                                                      \
> +               echo "ACTIVATE $(QEMU_VENV_DIR)";               \
> +               . $(QEMU_VENV_DIR)/bin/activate;                \
> +               echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";    \
> +               make develop 1>/dev/null;                       \
> +       )
> +       @touch $(QEMU_VENV_DIR)
> +
> +check-venv: venv
> +       @(                                                      \
> +               echo "ACTIVATE $(QEMU_VENV_DIR)";               \
> +               . $(QEMU_VENV_DIR)/bin/activate;                \
> +               make check;                                     \
> +       )
> +
>  develop:
> -       pip3 install -e .[devel]
> +       pip3 install --disable-pip-version-check -e .[devel]
>
>  check:
>         @avocado --config avocado.cfg run tests/
> @@ -50,4 +79,4 @@ clean:
>         python3 setup.py clean --all
>
>  distclean: clean
> -       rm -rf qemu.egg-info/ .venv/ .tox/ dist/
> +       rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/
> --
> 2.31.1
>
John Snow June 25, 2021, 6:38 p.m. UTC | #2
On 6/25/21 2:36 PM, Willian Rampazzo wrote:
> Maybe, it may confuse people using `make check-venv` under `tests`.
> Anyway, I'm not opposed to it.
> 
> Reviewed-by: Willian Rampazzo<willianr@redhat.com>

I have to admit there's much about Python packaging that is confusing :)

Can you elaborate on your point for me, though?

--js
Willian Rampazzo June 25, 2021, 7:01 p.m. UTC | #3
On Fri, Jun 25, 2021 at 3:38 PM John Snow <jsnow@redhat.com> wrote:
>
> On 6/25/21 2:36 PM, Willian Rampazzo wrote:
> > Maybe, it may confuse people using `make check-venv` under `tests`.
> > Anyway, I'm not opposed to it.
> >
> > Reviewed-by: Willian Rampazzo<willianr@redhat.com>
>
> I have to admit there's much about Python packaging that is confusing :)
>

Oh, the comment was not so related to packaging, but to QEMU itself.

> Can you elaborate on your point for me, though?

Under the `tests` folder, `make check-venv` creates the Python venv
for the tests. It does not run the tests. The `make check-venv` under
the `pyhton` folder proposed here will actually run the tests in the
venv. My comment was related to people already used to the behavior of
`make` under the `tests` folder.

I don't think it is this patch fault and I think this makes more sense
than what we currently have under the `tests` folder. Maybe it is just
a matter of organizing the `tests` folder `make` command.

>
> --js
>
John Snow June 25, 2021, 7:12 p.m. UTC | #4
On 6/25/21 3:01 PM, Willian Rampazzo wrote:
> On Fri, Jun 25, 2021 at 3:38 PM John Snow <jsnow@redhat.com> wrote:
>>
>> On 6/25/21 2:36 PM, Willian Rampazzo wrote:
>>> Maybe, it may confuse people using `make check-venv` under `tests`.
>>> Anyway, I'm not opposed to it.
>>>
>>> Reviewed-by: Willian Rampazzo<willianr@redhat.com>
>>
>> I have to admit there's much about Python packaging that is confusing :)
>>
> 
> Oh, the comment was not so related to packaging, but to QEMU itself.
> 
>> Can you elaborate on your point for me, though?
> 
> Under the `tests` folder, `make check-venv` creates the Python venv
> for the tests. It does not run the tests. The `make check-venv` under
> the `pyhton` folder proposed here will actually run the tests in the
> venv. My comment was related to people already used to the behavior of
> `make` under the `tests` folder.
> 
> I don't think it is this patch fault and I think this makes more sense
> than what we currently have under the `tests` folder. Maybe it is just
> a matter of organizing the `tests` folder `make` command.
> 

OH, I see what you're saying. It's the ambiguity between:

1. make [the] 'check' venv
2. make check, [with the] venv

I chose one semantic and tests/ chose another. Valid concern.

I suppose if someone does get it mixed up it won't hurt too much though, 
they'll certainly notice pretty quickly that 'check-venv' runs tests.

Cleber, my #1 co-maintainer, do you have any preferences here?

--js
Wainer dos Santos Moschetta June 28, 2021, 9:40 p.m. UTC | #5
On 6/25/21 12:45 PM, John Snow wrote:
> This is a *third* way to run the Python tests. Unlike the first two
> (check-pipenv, check-tox), this version does not require any specific
> interpreter version -- making it a lot easier to tell people to run it
> as a quick smoketest prior to submission to GitLab CI.
>
> Summary:
>
>    Checked via GitLab CI:
>      - check-pipenv: tests our oldest python & dependencies
>      - check-tox: tests newest dependencies on all non-EOL python versions
>    Executed only incidentally:
>      - check-venv: tests newest dependencies on whichever python version
>
> ('make check' does not set up any environment at all, it just runs the
> tests in your current environment. All four invocations perform the
> exact same tests, just in different execution environments.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/Makefile | 35 ++++++++++++++++++++++++++++++++---
>   1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/python/Makefile b/python/Makefile
> index 5cb8378b81..76bb24e671 100644
> --- a/python/Makefile
> +++ b/python/Makefile
> @@ -1,4 +1,6 @@
> -.PHONY: help pipenv check-pipenv check clean distclean develop
> +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
> +
btw, check-tox is missed here ^
> +QEMU_VENV_DIR=~/.cache/qemu-pyvenv

A few suggestions:

1. For the sake of consistence with others temporary directories 
created, use QEMU_VENV_DIR=<path-to-qemu-src>/.devvenv

2. Reword to 'devvenv' or 'dev-venv' (or something similar), instead of 
'venv', the directories and Make targets. IMHO it will make the purpose 
of the targets a bit clear.

What do you think John?

- Wainer

>   
>   help:
>   	@echo "python packaging help:"
> @@ -15,6 +17,11 @@ help:
>   	@echo "    Requires: Python 3.6-3.10 and tox."
>   	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
>   	@echo ""
> +	@echo "make check-venv:"
> +	@echo "    Run tests in a venv against your default python3 version."
> +	@echo "    These tests use the newest dependencies."
> +	@echo "    Requires: Python 3.x"
> +	@echo ""
>   	@echo "make develop:    Install deps for 'make check', and"
>   	@echo "                 the qemu libs in editable/development mode."
>   	@echo ""
> @@ -23,6 +30,9 @@ help:
>   	@echo "make pipenv"
>   	@echo "    Creates pipenv's virtual environment (.venv)"
>   	@echo ""
> +	@echo "make venv"
> +	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
> +	@echo ""
>   	@echo "make clean:      remove package build output."
>   	@echo ""
>   	@echo "make distclean:  remove venv files, qemu package forwarder,"
> @@ -37,8 +47,27 @@ pipenv: .venv
>   check-pipenv: pipenv
>   	@pipenv run make check
>   
> +venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
> +$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
> +	@echo "VENV $(QEMU_VENV_DIR)"
> +	@python3 -m venv $(QEMU_VENV_DIR)
> +	@(							\
> +		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
> +		. $(QEMU_VENV_DIR)/bin/activate;		\
> +		echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";	\
> +		make develop 1>/dev/null;			\
> +	)
> +	@touch $(QEMU_VENV_DIR)
> +
> +check-venv: venv
> +	@(							\
> +		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
> +		. $(QEMU_VENV_DIR)/bin/activate;		\
> +		make check;					\
> +	)
> +
>   develop:
> -	pip3 install -e .[devel]
> +	pip3 install --disable-pip-version-check -e .[devel]
>   
>   check:
>   	@avocado --config avocado.cfg run tests/
> @@ -50,4 +79,4 @@ clean:
>   	python3 setup.py clean --all
>   
>   distclean: clean
> -	rm -rf qemu.egg-info/ .venv/ .tox/ dist/
> +	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/
John Snow June 29, 2021, 3:45 p.m. UTC | #6
On Mon, Jun 28, 2021 at 5:40 PM Wainer dos Santos Moschetta <
wainersm@redhat.com> wrote:

>
> On 6/25/21 12:45 PM, John Snow wrote:
> > This is a *third* way to run the Python tests. Unlike the first two
> > (check-pipenv, check-tox), this version does not require any specific
> > interpreter version -- making it a lot easier to tell people to run it
> > as a quick smoketest prior to submission to GitLab CI.
> >
> > Summary:
> >
> >    Checked via GitLab CI:
> >      - check-pipenv: tests our oldest python & dependencies
> >      - check-tox: tests newest dependencies on all non-EOL python
> versions
> >    Executed only incidentally:
> >      - check-venv: tests newest dependencies on whichever python version
> >
> > ('make check' does not set up any environment at all, it just runs the
> > tests in your current environment. All four invocations perform the
> > exact same tests, just in different execution environments.)
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/Makefile | 35 ++++++++++++++++++++++++++++++++---
> >   1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/python/Makefile b/python/Makefile
> > index 5cb8378b81..76bb24e671 100644
> > --- a/python/Makefile
> > +++ b/python/Makefile
> > @@ -1,4 +1,6 @@
> > -.PHONY: help pipenv check-pipenv check clean distclean develop
> > +.PHONY: help pipenv venv check-venv check-pipenv check clean distclean
> develop
> > +
> btw, check-tox is missed here ^
>

Oops, thanks! I am not sure it winds up mattering, but writing Makefiles
feels like religion. I just follow the same steps and pray at the altar.


> > +QEMU_VENV_DIR=~/.cache/qemu-pyvenv
>
> A few suggestions:
>
> 1. For the sake of consistence with others temporary directories
> created, use QEMU_VENV_DIR=<path-to-qemu-src>/.devvenv
>
>
Sure, straight in this folder, like .tox and .venv you mean?

(I'd rename .venv to .pipenv, but pipenv doesn't let you name this folder,
annoyingly ...)


> 2. Reword to 'devvenv' or 'dev-venv' (or something similar), instead of
> 'venv', the directories and Make targets. IMHO it will make the purpose
> of the targets a bit clear.
>
>
Sure, I'll do that -- .dev-venv works for me.

>
> What do you think John?
>
>
I think that it's hot outside :)


> - Wainer
>

Thanks!
--js
diff mbox series

Patch

diff --git a/python/Makefile b/python/Makefile
index 5cb8378b81..76bb24e671 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -1,4 +1,6 @@ 
-.PHONY: help pipenv check-pipenv check clean distclean develop
+.PHONY: help pipenv venv check-venv check-pipenv check clean distclean develop
+
+QEMU_VENV_DIR=~/.cache/qemu-pyvenv
 
 help:
 	@echo "python packaging help:"
@@ -15,6 +17,11 @@  help:
 	@echo "    Requires: Python 3.6-3.10 and tox."
 	@echo "    Hint (Fedora): 'sudo dnf install python3-tox python3.10'"
 	@echo ""
+	@echo "make check-venv:"
+	@echo "    Run tests in a venv against your default python3 version."
+	@echo "    These tests use the newest dependencies."
+	@echo "    Requires: Python 3.x"
+	@echo ""
 	@echo "make develop:    Install deps for 'make check', and"
 	@echo "                 the qemu libs in editable/development mode."
 	@echo ""
@@ -23,6 +30,9 @@  help:
 	@echo "make pipenv"
 	@echo "    Creates pipenv's virtual environment (.venv)"
 	@echo ""
+	@echo "make venv"
+	@echo "    Creates a simple venv for check-venv. ($(QEMU_VENV_DIR))"
+	@echo ""
 	@echo "make clean:      remove package build output."
 	@echo ""
 	@echo "make distclean:  remove venv files, qemu package forwarder,"
@@ -37,8 +47,27 @@  pipenv: .venv
 check-pipenv: pipenv
 	@pipenv run make check
 
+venv: $(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate
+$(QEMU_VENV_DIR) $(QEMU_VENV_DIR)/bin/activate: setup.cfg
+	@echo "VENV $(QEMU_VENV_DIR)"
+	@python3 -m venv $(QEMU_VENV_DIR)
+	@(							\
+		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
+		. $(QEMU_VENV_DIR)/bin/activate;		\
+		echo "INSTALL qemu[devel] $(QEMU_VENV_DIR)";	\
+		make develop 1>/dev/null;			\
+	)
+	@touch $(QEMU_VENV_DIR)
+
+check-venv: venv
+	@(							\
+		echo "ACTIVATE $(QEMU_VENV_DIR)";		\
+		. $(QEMU_VENV_DIR)/bin/activate;		\
+		make check;					\
+	)
+
 develop:
-	pip3 install -e .[devel]
+	pip3 install --disable-pip-version-check -e .[devel]
 
 check:
 	@avocado --config avocado.cfg run tests/
@@ -50,4 +79,4 @@  clean:
 	python3 setup.py clean --all
 
 distclean: clean
-	rm -rf qemu.egg-info/ .venv/ .tox/ dist/
+	rm -rf qemu.egg-info/ .venv/ .tox/ $(QEMU_VENV_DIR) dist/