[pacman-dev] pactest: add environment option to run tests with valgrind

Message ID 20191126002644.731185-1-eschwartz@archlinux.org
State Accepted, archived
Headers show
Series [pacman-dev] pactest: add environment option to run tests with valgrind | expand

Commit Message

Eli Schwartz Nov. 26, 2019, 12:26 a.m. UTC
In autotools, if we wanted to run tests with valgrind, we used some Make
magic which passed arguments to pactest.py, but that doesn't work in
meson, because all arguments are encoded at configure time. Instead,
let's short-circuit the build runner logic entirely, and teach pactest
to default to running valgrind, when it detects an environment variable
set independent of the build system.

To run the tests with valgrind, we can now use:

PACTEST_VALGRIND=1 meson test -C builddir/

or

PACTEST_VALGRIND=1 make check

It is also possible, but confusing/inconsistent, to use

make check PY_LOG_FLAGS=--valgrind

We *could* add a meson option -Dvalgrind=true, but that is annoying to
reconfigure between test runs, and overall the consensus is it seems
simpler to opt in each time we want to run valgrind, as was already the
case.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 test/pacman/pactest.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Allan McRae Nov. 26, 2019, 1:08 a.m. UTC | #1
On 26/11/19 10:26 am, Eli Schwartz wrote:
> In autotools, if we wanted to run tests with valgrind, we used some Make
> magic which passed arguments to pactest.py, but that doesn't work in
> meson, because all arguments are encoded at configure time. Instead,
> let's short-circuit the build runner logic entirely, and teach pactest
> to default to running valgrind, when it detects an environment variable
> set independent of the build system.
> 
> To run the tests with valgrind, we can now use:
> 
> PACTEST_VALGRIND=1 meson test -C builddir/
> 
> or
> 
> PACTEST_VALGRIND=1 make check
> 
> It is also possible, but confusing/inconsistent, to use
> 
> make check PY_LOG_FLAGS=--valgrind
> 
> We *could* add a meson option -Dvalgrind=true, but that is annoying to
> reconfigure between test runs, and overall the consensus is it seems
> simpler to opt in each time we want to run valgrind, as was already the
> case.
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>

Works as advertised!   Thanks.

A

> ---
>  test/pacman/pactest.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
> index a5891d17..3414f4cc 100755
> --- a/test/pacman/pactest.py
> +++ b/test/pacman/pactest.py
> @@ -96,7 +96,7 @@ def create_parser():
>                        dest = "gdb", default = False,
>                        help = "use gdb while calling pacman")
>      parser.add_option("--valgrind", action = "store_true",
> -                      dest = "valgrind", default = False,
> +                      dest = "valgrind", default = os.getenv('PACTEST_VALGRIND'),
>                        help = "use valgrind while calling pacman")
>      parser.add_option("--manual-confirm", action = "store_true",
>                        dest = "manualconfirm", default = False,
>

Patch

diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index a5891d17..3414f4cc 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -96,7 +96,7 @@  def create_parser():
                       dest = "gdb", default = False,
                       help = "use gdb while calling pacman")
     parser.add_option("--valgrind", action = "store_true",
-                      dest = "valgrind", default = False,
+                      dest = "valgrind", default = os.getenv('PACTEST_VALGRIND'),
                       help = "use valgrind while calling pacman")
     parser.add_option("--manual-confirm", action = "store_true",
                       dest = "manualconfirm", default = False,