tests, python: prepare to expand usage of test venv

[PATCH 9/9] tests: run 'device-crash-test' from tests/venv

Posted by John Snow 3 years, 8 months ago

Remove the sys.path hacking from device-crash-test, and add in a little
user-friendly message for anyone who was used to running this script
directly from the source tree.

Modify the GitLab job recipes to create the tests/venv first, then run
device-crash-test from that venv.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 .gitlab-ci.d/buildtest.yml |  8 +++++---
 scripts/device-crash-test  | 14 +++++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index e9620c30748..fde29c35aa3 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -110,7 +110,8 @@ crash-test-debian:
     IMAGE: debian-amd64
   script:
     - cd build
-    - scripts/device-crash-test -q ./qemu-system-i386
+    - make check-venv
+    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-i386
 
 build-system-fedora:
   extends: .native_build_job_template
@@ -155,8 +156,9 @@ crash-test-fedora:
     IMAGE: fedora
   script:
     - cd build
-    - scripts/device-crash-test -q ./qemu-system-ppc
-    - scripts/device-crash-test -q ./qemu-system-riscv32
+    - make check-venv
+    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc
+    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32
 
 build-system-centos:
   extends: .native_build_job_template
diff --git a/scripts/device-crash-test b/scripts/device-crash-test
index a203b3fdea2..73bcb986937 100755
--- a/scripts/device-crash-test
+++ b/scripts/device-crash-test
@@ -33,10 +33,18 @@ import re
 import random
 import argparse
 from itertools import chain
+from pathlib import Path
 
-sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
-from qemu.machine import QEMUMachine
-from qemu.qmp import ConnectError
+try:
+    from qemu.machine import QEMUMachine
+    from qemu.qmp import ConnectError
+except ModuleNotFoundError as exc:
+    path = Path(__file__).resolve()
+    print(f"Module '{exc.name}' not found.")
+    print("  Try 'make check-venv' from your build directory,")
+    print("  and then one way to run this script is like so:")
+    print(f'  > $builddir/tests/venv/bin/python3 "{path}"')
+    sys.exit(1)
 
 logger = logging.getLogger('device-crash-test')
 dbg = logger.debug
-- 
2.34.1

Re: [PATCH 9/9] tests: run 'device-crash-test' from tests/venv

Posted by Paolo Bonzini 3 years, 8 months ago

On 5/26/22 02:09, John Snow wrote:
> Remove the sys.path hacking from device-crash-test, and add in a little
> user-friendly message for anyone who was used to running this script
> directly from the source tree.
> 
> Modify the GitLab job recipes to create the tests/venv first, then run
> device-crash-test from that venv.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   .gitlab-ci.d/buildtest.yml |  8 +++++---
>   scripts/device-crash-test  | 14 +++++++++++---
>   2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index e9620c30748..fde29c35aa3 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -110,7 +110,8 @@ crash-test-debian:
>       IMAGE: debian-amd64
>     script:
>       - cd build
> -    - scripts/device-crash-test -q ./qemu-system-i386
> +    - make check-venv
> +    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-i386
>   
>   build-system-fedora:
>     extends: .native_build_job_template
> @@ -155,8 +156,9 @@ crash-test-fedora:
>       IMAGE: fedora
>     script:
>       - cd build
> -    - scripts/device-crash-test -q ./qemu-system-ppc
> -    - scripts/device-crash-test -q ./qemu-system-riscv32
> +    - make check-venv
> +    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc
> +    - tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32
>   
>   build-system-centos:
>     extends: .native_build_job_template
> diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> index a203b3fdea2..73bcb986937 100755
> --- a/scripts/device-crash-test
> +++ b/scripts/device-crash-test
> @@ -33,10 +33,18 @@ import re
>   import random
>   import argparse
>   from itertools import chain
> +from pathlib import Path
>   
> -sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> -from qemu.machine import QEMUMachine
> -from qemu.qmp import ConnectError
> +try:
> +    from qemu.machine import QEMUMachine
> +    from qemu.qmp import ConnectError
> +except ModuleNotFoundError as exc:
> +    path = Path(__file__).resolve()
> +    print(f"Module '{exc.name}' not found.")
> +    print("  Try 'make check-venv' from your build directory,")
> +    print("  and then one way to run this script is like so:")
> +    print(f'  > $builddir/tests/venv/bin/python3 "{path}"')
> +    sys.exit(1)
>   
>   logger = logging.getLogger('device-crash-test')
>   dbg = logger.debug

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Even though I'd still prefer the venv to be setup early (so the 
check-venv change in buildtest.yml and the friendly message in the 
script will go away), this is a step in the right direction.

Paolo

Re: [PATCH 9/9] tests: run 'device-crash-test' from tests/venv

Posted by John Snow 3 years, 8 months ago

On Thu, May 26, 2022, 8:14 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 5/26/22 02:09, John Snow wrote:
> > Remove the sys.path hacking from device-crash-test, and add in a little
> > user-friendly message for anyone who was used to running this script
> > directly from the source tree.
> >
> > Modify the GitLab job recipes to create the tests/venv first, then run
> > device-crash-test from that venv.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   .gitlab-ci.d/buildtest.yml |  8 +++++---
> >   scripts/device-crash-test  | 14 +++++++++++---
> >   2 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> > index e9620c30748..fde29c35aa3 100644
> > --- a/.gitlab-ci.d/buildtest.yml
> > +++ b/.gitlab-ci.d/buildtest.yml
> > @@ -110,7 +110,8 @@ crash-test-debian:
> >       IMAGE: debian-amd64
> >     script:
> >       - cd build
> > -    - scripts/device-crash-test -q ./qemu-system-i386
> > +    - make check-venv
> > +    - tests/venv/bin/python3 scripts/device-crash-test -q
> ./qemu-system-i386
> >
> >   build-system-fedora:
> >     extends: .native_build_job_template
> > @@ -155,8 +156,9 @@ crash-test-fedora:
> >       IMAGE: fedora
> >     script:
> >       - cd build
> > -    - scripts/device-crash-test -q ./qemu-system-ppc
> > -    - scripts/device-crash-test -q ./qemu-system-riscv32
> > +    - make check-venv
> > +    - tests/venv/bin/python3 scripts/device-crash-test -q
> ./qemu-system-ppc
> > +    - tests/venv/bin/python3 scripts/device-crash-test -q
> ./qemu-system-riscv32
> >
> >   build-system-centos:
> >     extends: .native_build_job_template
> > diff --git a/scripts/device-crash-test b/scripts/device-crash-test
> > index a203b3fdea2..73bcb986937 100755
> > --- a/scripts/device-crash-test
> > +++ b/scripts/device-crash-test
> > @@ -33,10 +33,18 @@ import re
> >   import random
> >   import argparse
> >   from itertools import chain
> > +from pathlib import Path
> >
> > -sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
> > -from qemu.machine import QEMUMachine
> > -from qemu.qmp import ConnectError
> > +try:
> > +    from qemu.machine import QEMUMachine
> > +    from qemu.qmp import ConnectError
> > +except ModuleNotFoundError as exc:
> > +    path = Path(__file__).resolve()
> > +    print(f"Module '{exc.name}' not found.")
> > +    print("  Try 'make check-venv' from your build directory,")
> > +    print("  and then one way to run this script is like so:")
> > +    print(f'  > $builddir/tests/venv/bin/python3 "{path}"')
> > +    sys.exit(1)
> >
> >   logger = logging.getLogger('device-crash-test')
> >   dbg = logger.debug
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> Even though I'd still prefer the venv to be setup early (so the
> check-venv change in buildtest.yml and the friendly message in the
> script will go away), this is a step in the right direction.
>
> Paolo
>

Agree, figured I'd do baby steps before I wound up with a 40 patch series,
and this gives Thomas et al a chance to find out if this ruins their
workflow.

(I'll probably keep the friendly message a little while more anyway,
though; to catch anyone who runs this script manually for a release or so.
I should add a section to our QEMU developer's guide and just link to it in
the message and explain the many, many ways you might enter a venv or
install the package.)