[RFC PATCH 9/9] iotests: use tests/venv for running tests

John Snow posted 9 patches 3 years, 9 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by John Snow 3 years, 9 months ago
Essentially, this:

(A) adjusts the python binary to be the one found in the venv (which is
a symlink to the python binary chosen at configure time)

(B) adds a new VIRTUAL_ENV export variable

(C) changes PATH to front-load the venv binary directory.

If the venv directory isn't found, raise a friendly exception that tries
to give the human operator a friendly clue as to what's gone wrong. In
the very near future, I'd like to teach iotests how to fix this problem
entirely of its own volition, but that's a trick for a little later.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 0007da3f06c..fd3720ed7e7 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
     # lot of them. Silence pylint:
     # pylint: disable=too-many-instance-attributes
 
-    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
-                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
+                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+                     'QEMU_PROG', 'QEMU_IMG_PROG',
                      'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
                      'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
                      'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
@@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
             if val is not None:
                 env[v] = val
 
+        env['PATH'] = os.pathsep.join((
+            os.path.join(self.virtual_env, 'bin'),
+            os.environ['PATH']
+        ))
         return env
 
     def init_directories(self) -> None:
@@ -107,13 +112,17 @@ def init_directories(self) -> None:
              SOCK_DIR
              SAMPLE_IMG_DIR
         """
-
-        # Path where qemu goodies live in this source tree.
-        qemu_srctree_path = Path(__file__, '../../../python').resolve()
+        venv_path = Path(self.build_root, 'tests/venv/')
+        if not venv_path.exists():
+            raise FileNotFoundError(
+                f"Virtual environment \"{venv_path!s}\" isn't found."
+                " (Maybe you need to run 'make check-venv'"
+                " from the build dir?)"
+            )
+        self.virtual_env: str = str(venv_path)
 
         self.pythonpath = os.pathsep.join(filter(None, (
             self.source_iotests,
-            str(qemu_srctree_path),
             os.getenv('PYTHONPATH'),
         )))
 
@@ -138,7 +147,7 @@ def init_binaries(self) -> None:
              PYTHON (for bash tests)
              QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
         """
-        self.python = sys.executable
+        self.python: str = os.path.join(self.virtual_env, 'bin', 'python3')
 
         def root(*names: str) -> str:
             return os.path.join(self.build_root, *names)
@@ -300,6 +309,7 @@ def print_env(self, prefix: str = '') -> None:
 {prefix}GDB_OPTIONS   -- {GDB_OPTIONS}
 {prefix}VALGRIND_QEMU -- {VALGRIND_QEMU}
 {prefix}PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
+{prefix}VIRTUAL_ENV   -- {VIRTUAL_ENV}
 {prefix}"""
 
         args = collections.defaultdict(str, self.get_env())
-- 
2.34.1
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by Paolo Bonzini 3 years, 9 months ago
On 5/13/22 02:06, John Snow wrote:
> Essentially, this:
> 
> (A) adjusts the python binary to be the one found in the venv (which is
> a symlink to the python binary chosen at configure time)
> 
> (B) adds a new VIRTUAL_ENV export variable
> 
> (C) changes PATH to front-load the venv binary directory.
> 
> If the venv directory isn't found, raise a friendly exception that tries
> to give the human operator a friendly clue as to what's gone wrong. In
> the very near future, I'd like to teach iotests how to fix this problem
> entirely of its own volition, but that's a trick for a little later.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 0007da3f06c..fd3720ed7e7 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
>       # lot of them. Silence pylint:
>       # pylint: disable=too-many-instance-attributes
>   
> -    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> -                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> +    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
> +                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> +                     'QEMU_PROG', 'QEMU_IMG_PROG',
>                        'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
>                        'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
>                        'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
> @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
>               if val is not None:
>                   env[v] = val
>   
> +        env['PATH'] = os.pathsep.join((
> +            os.path.join(self.virtual_env, 'bin'),
> +            os.environ['PATH']
> +        ))
>           return env
>   
>       def init_directories(self) -> None:
> @@ -107,13 +112,17 @@ def init_directories(self) -> None:
>                SOCK_DIR
>                SAMPLE_IMG_DIR
>           """
> -
> -        # Path where qemu goodies live in this source tree.
> -        qemu_srctree_path = Path(__file__, '../../../python').resolve()
> +        venv_path = Path(self.build_root, 'tests/venv/')
> +        if not venv_path.exists():
> +            raise FileNotFoundError(
> +                f"Virtual environment \"{venv_path!s}\" isn't found."
> +                " (Maybe you need to run 'make check-venv'"
> +                " from the build dir?)"
> +            )
> +        self.virtual_env: str = str(venv_path)
>   
>           self.pythonpath = os.pathsep.join(filter(None, (
>               self.source_iotests,
> -            str(qemu_srctree_path),
>               os.getenv('PYTHONPATH'),
>           )))
>   
> @@ -138,7 +147,7 @@ def init_binaries(self) -> None:
>                PYTHON (for bash tests)
>                QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
>           """
> -        self.python = sys.executable
> +        self.python: str = os.path.join(self.virtual_env, 'bin', 'python3')

Is this guaranteed even if, say, only a /usr/bin/python3.9 exists? 
os.path.basename(sys.executable) might be more weirdness-proof than 
'python3'.

Paolo
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by John Snow 3 years, 9 months ago
On Fri, May 13, 2022, 4:38 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 02:06, John Snow wrote:
> > Essentially, this:
> >
> > (A) adjusts the python binary to be the one found in the venv (which is
> > a symlink to the python binary chosen at configure time)
> >
> > (B) adds a new VIRTUAL_ENV export variable
> >
> > (C) changes PATH to front-load the venv binary directory.
> >


(amending my commit message/rfc notes while I'm here:)

I'll add that this way of entering a venv is more complex than the method
used for VM tests and Avocado tests because it allows the possibility of
shell tests (et al) invoking python utilities and having those be "in the
venv" as well.

i.e. it's more rigorous and it matches the behavior of the "activate" shell
script bundled with every venv.


> > If the venv directory isn't found, raise a friendly exception that tries
> > to give the human operator a friendly clue as to what's gone wrong. In
> > the very near future, I'd like to teach iotests how to fix this problem
> > entirely of its own volition, but that's a trick for a little later.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/qemu-iotests/testenv.py | 24 +++++++++++++++++-------
> >   1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 0007da3f06c..fd3720ed7e7 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -65,8 +65,9 @@ class TestEnv(ContextManager['TestEnv']):
> >       # lot of them. Silence pylint:
> >       # pylint: disable=too-many-instance-attributes
> >
> > -    env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR',
> 'SAMPLE_IMG_DIR',
> > -                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> > +    env_variables = ['PYTHONPATH', 'VIRTUAL_ENV', 'PYTHON',
> > +                     'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> > +                     'QEMU_PROG', 'QEMU_IMG_PROG',
> >                        'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> >                        'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
> >                        'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
> > @@ -98,6 +99,10 @@ def get_env(self) -> Dict[str, str]:
> >               if val is not None:
> >                   env[v] = val
> >
> > +        env['PATH'] = os.pathsep.join((
> > +            os.path.join(self.virtual_env, 'bin'),
> > +            os.environ['PATH']
> > +        ))
> >           return env
> >
> >       def init_directories(self) -> None:
> > @@ -107,13 +112,17 @@ def init_directories(self) -> None:
> >                SOCK_DIR
> >                SAMPLE_IMG_DIR
> >           """
> > -
> > -        # Path where qemu goodies live in this source tree.
> > -        qemu_srctree_path = Path(__file__, '../../../python').resolve()
> > +        venv_path = Path(self.build_root, 'tests/venv/')
> > +        if not venv_path.exists():
> > +            raise FileNotFoundError(
> > +                f"Virtual environment \"{venv_path!s}\" isn't found."
> > +                " (Maybe you need to run 'make check-venv'"
> > +                " from the build dir?)"
> > +            )
> > +        self.virtual_env: str = str(venv_path)
> >
> >           self.pythonpath = os.pathsep.join(filter(None, (
> >               self.source_iotests,
> > -            str(qemu_srctree_path),
> >               os.getenv('PYTHONPATH'),
> >           )))
> >
> > @@ -138,7 +147,7 @@ def init_binaries(self) -> None:
> >                PYTHON (for bash tests)
> >                QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG,
> QSD_PROG
> >           """
> > -        self.python = sys.executable
> > +        self.python: str = os.path.join(self.virtual_env, 'bin',
> 'python3')
>
> Is this guaranteed even if, say, only a /usr/bin/python3.9 exists?
> os.path.basename(sys.executable) might be more weirdness-proof than
> 'python3'.
>
> Paolo
>

It *should*, because "#!/usr/bin/env python3" is the preferred shebang for
Python scripts.

https://peps.python.org/pep-0394/

'python3' "should" be available. 'python' may not be.

Probably the "python" name in Makefile for TESTS_PYTHON should actually be
"python3" as well. In practice, all permutations (python, python3,
python3.9, etc.) are symlinks* to the binary used to create the venv. Which
links are present may be site configurable, but pep394 should guarantee
that python3 is always available.

(* not on Windows, where it'll be a copy.)

>
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by Paolo Bonzini 3 years, 9 months ago
On 5/13/22 16:38, John Snow wrote:
> It *should*, because "#!/usr/bin/env python3" is the preferred shebang 
> for Python scripts.
> 
> https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> 
> 'python3' "should" be available. 'python' may not be.
> 
> Probably the "python" name in Makefile for TESTS_PYTHON should actually 
> be "python3" as well. In practice, all permutations (python, python3, 
> python3.9, etc.) are symlinks* to the binary used to create the venv. 
> Which links are present may be site configurable, but pep394 should 
> guarantee that python3 is always available.

IIRC we have some cases (FreeBSD?) where only the python3.x executable 
is available.  This is why we 1) default to Meson's Python 3 if neither 
--meson nor --python are passed, and 2) use the shebang you mention but 
with *non-executable* files, which Meson treats magically as "invoke 
with the Python interpreter that was used to launch me".

Paolo

Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by John Snow 3 years, 9 months ago
On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:38, John Snow wrote:
> > It *should*, because "#!/usr/bin/env python3" is the preferred shebang
> > for Python scripts.
> >
> > https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> >
> > 'python3' "should" be available. 'python' may not be.
> >
> > Probably the "python" name in Makefile for TESTS_PYTHON should actually
> > be "python3" as well. In practice, all permutations (python, python3,
> > python3.9, etc.) are symlinks* to the binary used to create the venv.
> > Which links are present may be site configurable, but pep394 should
> > guarantee that python3 is always available.
>
> IIRC we have some cases (FreeBSD?) where only the python3.x executable
> is available.  This is why we 1) default to Meson's Python 3 if neither
> --meson nor --python are passed, and 2) use the shebang you mention but
> with *non-executable* files, which Meson treats magically as "invoke
> with the Python interpreter that was used to launch me".
>
> Paolo
>

pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do you
know in what circumstances you get only a point release binary?

Creating a venv on fbsd with "python3 -m venv testvenv" created a python3
binary link, but not a python3.8 link, also.

Still leaning towards the idea that "python3" is safest, but maybe it
depends on how you install from ports etc. I'd still say that it's
reasonable to expect that a system with python pays heed to PEP0394, I
think you've got a broken python install if you don't.

(But, what's the use case that forced your hand otherwise?)

--js

>
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by Paolo Bonzini 3 years, 8 months ago
On 5/14/22 17:55, John Snow wrote:
> On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com 
> <mailto:pbonzini@redhat.com>> wrote:
>     IIRC we have some cases (FreeBSD?) where only the python3.x executable
>     is available.  This is why we 1) default to Meson's Python 3 if neither
>     --meson nor --python are passed, and 2) use the shebang you mention but
>     with *non-executable* files, which Meson treats magically as "invoke
>     with the Python interpreter that was used to launch me".
> 
> pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do 
> you know in what circumstances you get only a point release binary?

Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's 
still a plausible configuration for this packaging setup.

Paolo

> Creating a venv on fbsd with "python3 -m venv testvenv" created a 
> python3 binary link, but not a python3.8 link, also.
> 
> Still leaning towards the idea that "python3" is safest, but maybe it 
> depends on how you install from ports etc. I'd still say that it's 
> reasonable to expect that a system with python pays heed to PEP0394, I 
> think you've got a broken python install if you don't.
> 
> (But, what's the use case that forced your hand otherwise?)
> 
> --js
> 


Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by John Snow 3 years, 8 months ago
On Mon, May 16, 2022 at 3:41 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/14/22 17:55, John Snow wrote:
> > On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com
> > <mailto:pbonzini@redhat.com>> wrote:
> >     IIRC we have some cases (FreeBSD?) where only the python3.x executable
> >     is available.  This is why we 1) default to Meson's Python 3 if neither
> >     --meson nor --python are passed, and 2) use the shebang you mention but
> >     with *non-executable* files, which Meson treats magically as "invoke
> >     with the Python interpreter that was used to launch me".
> >
> > pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do
> > you know in what circumstances you get only a point release binary?
>
> Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's
> still a plausible configuration for this packaging setup.
>

Just confirming here that if you do 'pkg install python37' and you
have no 'python3' link, the venv package will still make 'python' and
'python3' links. I think it's likely best to use the 'python3' one.
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by Paolo Bonzini 3 years, 8 months ago
On 5/18/22 01:51, John Snow wrote:
>>> pkg install python3 on fbsd 13.0-R gives you /usr/bin/python3 fwiw. do
>>> you know in what circumstances you get only a point release binary?
>> Aha, tests/vm/freebsd installs python37, not python3.  But I guess it's
>> still a plausible configuration for this packaging setup.
>>
> Just confirming here that if you do 'pkg install python37' and you
> have no 'python3' link, the venv package will still make 'python' and
> 'python3' links. I think it's likely best to use the 'python3' one.

Ok, another good reason to always go through a venv.

Paolo
Re: [RFC PATCH 9/9] iotests: use tests/venv for running tests
Posted by John Snow 3 years, 9 months ago
On Fri, May 13, 2022, 11:33 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/13/22 16:38, John Snow wrote:
> > It *should*, because "#!/usr/bin/env python3" is the preferred shebang
> > for Python scripts.
> >
> > https://peps.python.org/pep-0394/ <https://peps.python.org/pep-0394/>
> >
> > 'python3' "should" be available. 'python' may not be.
> >
> > Probably the "python" name in Makefile for TESTS_PYTHON should actually
> > be "python3" as well. In practice, all permutations (python, python3,
> > python3.9, etc.) are symlinks* to the binary used to create the venv.
> > Which links are present may be site configurable, but pep394 should
> > guarantee that python3 is always available.
>
> IIRC we have some cases (FreeBSD?) where only the python3.x executable
> is available.  This is why we 1) default to Meson's Python 3 if neither
> --meson nor --python are passed, and 2) use the shebang you mention but
> with *non-executable* files, which Meson treats magically as "invoke
> with the Python interpreter that was used to launch me".
>

This tidbit is particularly 😥


> Paolo
>

FreeBSD, why do you insist on hurting me?

(I'm surprised - python3 is *supposed* to be defined. Isn't this supremely
annoying for FreeBSD users to have every last Python shebang script not
work?)

OK. I'll test, and possibly update avocado's existing makefile magic if
necessary. It may be the case that the venv just works anyway. No way to
know but to test, I guess.