GNOME.org
 

[gjs] build: Valgrind with AX_VALGRIND_CHECK

commit 73beb1e599078874de6137cd858b896cc286c9b9
Author: Philip Chimento <philip endlessm com>
Date:   Wed Aug 30 22:46:10 2017 -0700

    build: Valgrind with AX_VALGRIND_CHECK
    
    This adds a "make check-valgrind" target which will run the test suite
    under two Valgrind tools in succession: memcheck and helgrind. Requires
    a bit of surgery on the shell script test runners, because
    AX_VALGRIND_CHECK mainly assumes your tests are binaries.
    
    The Valgrind tools drd and sgcheck are also available, but disabled by
    default because they seem to crash on SpiderMonkey's threading.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786995

 Makefile-test.am                           |   10 +++++-
 configure.ac                               |    5 +++
 doc/Hacking.md                             |   19 ++++------
 installed-tests/scripts/testCommandLine.sh |   50 ++++++++++++++--------------
 installed-tests/scripts/testWarnings.sh    |    8 ++--
 test/run-test                              |    2 +-
 6 files changed, 52 insertions(+), 42 deletions(-)
---
diff --git a/Makefile-test.am b/Makefile-test.am
index e5e94e1..8cfaa32 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -264,11 +264,18 @@ EXTRA_DIST +=                                             \
        installed-tests/js/testCairo.js                 \
        installed-tests/js/testGtk.js                   \
        installed-tests/js/testGDBus.js                 \
+       installed-tests/extra/gjs.supp                  \
        installed-tests/extra/lsan.supp                 \
        $(NULL)
 
 ### TEST EXECUTION #####################################################
 
+@VALGRIND_CHECK_RULES@
+VALGRIND_SUPPRESSIONS_FILES =                          \
+       $(datadir)/glib-2.0/valgrind/glib.supp          \
+       $(top_srcdir)/installed-tests/extra/gjs.supp    \
+       $(NULL)
+
 if DBUS_TESTS
 DBUS_SESSION_COMMAND = $(DBUS_RUN_SESSION) --config-file=$(srcdir)/test/test-bus.conf --
 else
@@ -294,6 +301,7 @@ AM_TESTS_ENVIRONMENT =                                      \
        export LD_LIBRARY_PATH="$(builddir)/.libs:$${LD_LIBRARY_PATH:+:$$LD_LIBRARY_PATH}"; \
        export G_FILENAME_ENCODING=latin1;              \
        export LSAN_OPTIONS="suppressions=$(abs_top_srcdir)/installed-tests/extra/lsan.supp"; \
+       export NO_AT_BRIDGE=1;                          \
        $(COVERAGE_TESTS_ENVIRONMENT)                   \
        $(XVFB_START)                                   \
        $(DBUS_SESSION_COMMAND)                         \
@@ -321,7 +329,7 @@ GTESTER_LOG_COMPILER = $(top_srcdir)/test/run-test
 SH_LOG_DRIVER = $(LOG_DRIVER)
 
 JS_LOG_DRIVER = $(LOG_DRIVER)
-JS_LOG_COMPILER = $(top_builddir)/minijasmine
+JS_LOG_COMPILER = $$LOG_COMPILER $$LOG_FLAGS $(top_builddir)/minijasmine
 
 CODE_COVERAGE_IGNORE_PATTERN = */{include,mfbt,gjs/test,gjs/installed-tests}/*
 CODE_COVERAGE_GENHTML_OPTIONS =                        \
diff --git a/configure.ac b/configure.ac
index 6bbf733..84bba8e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -52,6 +52,11 @@ AX_CHECK_LINK_FLAG([-lgcov],, [
        CODE_COVERAGE_LDFLAGS=
 ])
 
+AX_VALGRIND_DFLT([drd], [off])
+AX_VALGRIND_DFLT([sgcheck], [off])
+# DRD and sgcheck don't produce useful diagnostics at this time
+AX_VALGRIND_CHECK
+
 # Checks for libraries.
 m4_define(glib_required_version, 2.50.0)
 
diff --git a/doc/Hacking.md b/doc/Hacking.md
index 51c18c0..2987b22 100644
--- a/doc/Hacking.md
+++ b/doc/Hacking.md
@@ -75,22 +75,19 @@ not being updated after GC moved it.
 ### Valgrind ###
 
 Valgrind catches memory leak errors in the C++ code.
-It's a good idea to run the test suite under Valgrind every once in a
-while.
+It's a good idea to run the test suite under Valgrind before each
+release.
 
-Download the Valgrind false-positive suppression file from GLib:
+To run the test suite under a succession of Valgrind tools:
 ```sh
-wget https://git.gnome.org/browse/glib/plain/glib.supp
+jhbuild make check-valgrind
 ```
 
-Run the test suite:
-```sh
-jhbuild shell
-cd ~/.cache/jhbuild/build/gjs
-G_DEBUG=gc-friendly G_SLICE=always-malloc ./libtool --mode=execute valgrind --leak-check=yes 
--suppressions=/path/to/glib.supp ./gjs-tests
-```
+The logs from each run will be in `~/.cache/jhbuild/build/gjs/test-suite-<toolname>.log`, where `<toolname>` 
is `drd`, `helgrind`, `memcheck`, and `sgcheck`.
 
-(And a similar command to run each `minijasmine` test.)
+Note that LeakSanitizer, part of ASan (see below) can catch many, but
+not all, errors that Valgrind can catch.
+LSan executes faster than Valgrind, however.
 
 ### Static Code Analysis ###
 
diff --git a/installed-tests/scripts/testCommandLine.sh b/installed-tests/scripts/testCommandLine.sh
index 74ce17d..df9c6e6 100755
--- a/installed-tests/scripts/testCommandLine.sh
+++ b/installed-tests/scripts/testCommandLine.sh
@@ -1,9 +1,9 @@
 #!/bin/sh
 
 if test "$GJS_USE_UNINSTALLED_FILES" = "1"; then
-    gjs="$TOP_BUILDDIR/gjs-console"
+    gjs="$LOG_COMPILER $LOG_FLAGS $TOP_BUILDDIR/gjs-console"
 else
-    gjs="gjs-console"
+    gjs="$LOG_COMPILER $LOG_FLAGS gjs-console"
 fi
 
 # This JS script should exit immediately with code 42. If that is not working,
@@ -73,52 +73,52 @@ report_xfail () {
 }
 
 # Test that System.exit() works in gjs-console
-"$gjs" -c 'imports.system.exit(0)'
+$gjs -c 'imports.system.exit(0)'
 report "System.exit(0) should exit successfully"
-"$gjs" -c 'imports.system.exit(42)'
+$gjs -c 'imports.system.exit(42)'
 test $? -eq 42
 report "System.exit(42) should exit with the correct exit code"
 
 # FIXME: should check -eq 42 specifically, but in debug mode we will be
 # hitting an assertion
-"$gjs" exit.js
+$gjs exit.js
 test $? -ne 0
 report "System.exit() should still exit across an FFI boundary"
 
 # gjs --help prints GJS help
-"$gjs" --help >/dev/null
+$gjs --help >/dev/null
 report "--help should succeed"
-test -n "$("$gjs" --help)"
+test -n "$($gjs --help)"
 report "--help should print something"
 
 # print GJS help even if it's not the first argument
-"$gjs" -I . --help >/dev/null
+$gjs -I . --help >/dev/null
 report "should succeed when --help is not first arg"
-test -n "$("$gjs" -I . --help)"
+test -n "$($gjs -I . --help)"
 report "should print something when --help is not first arg"
 
 # --help before a script file name prints GJS help
-"$gjs" --help help.js >/dev/null
+$gjs --help help.js >/dev/null
 report "--help should succeed before a script file"
-test -n "$("$gjs" --help help.js)"
+test -n "$($gjs --help help.js)"
 report "--help should print something before a script file"
 
 # --help before a -c argument prints GJS help
 script='imports.system.exit(1)'
-"$gjs" --help -c "$script" >/dev/null
+$gjs --help -c "$script" >/dev/null
 report "--help should succeed before -c"
-test -n "$("$gjs" --help -c "$script")"
+test -n "$($gjs --help -c "$script")"
 report "--help should print something before -c"
 
 # --help after a script file name is passed to the script
-"$gjs" -I sentinel help.js --help
+$gjs -I sentinel help.js --help
 report "--help after script file should be passed to script"
 test -z "$("$gjs" -I sentinel help.js --help)"
 report "--help after script file should not print anything"
 
 # --help after a -c argument is passed to the script
 script='if(ARGV[0] !== "--help") imports.system.exit(1)'
-"$gjs" -c "$script" --help
+$gjs -c "$script" --help
 report "--help after -c should be passed to script"
 test -z "$("$gjs" -c "$script" --help)"
 report "--help after -c should not print anything"
@@ -128,29 +128,29 @@ report "--help after -c should not print anything"
 # "$gjs" help.js --help -I sentinel
 # report_xfail "-I after script file should not be added to search path"
 # fi
-"$gjs" help.js --help -I sentinel 2>&1 | grep -q 'Gjs-WARNING.*--include-path'
+$gjs help.js --help -I sentinel 2>&1 | grep -q 'Gjs-WARNING.*--include-path'
 report "-I after script should succeed but give a warning"
-"$gjs" -c 'imports.system.exit(0)' --coverage-prefix=foo --coverage-output=foo 2>&1 | grep -q 
'Gjs-WARNING.*--coverage-prefix'
+$gjs -c 'imports.system.exit(0)' --coverage-prefix=foo --coverage-output=foo 2>&1 | grep -q 
'Gjs-WARNING.*--coverage-prefix'
 report "--coverage-prefix after script should succeed but give a warning"
-"$gjs" -c 'imports.system.exit(0)' --coverage-prefix=foo --coverage-output=foo 2>&1 | grep -q 
'Gjs-WARNING.*--coverage-output'
+$gjs -c 'imports.system.exit(0)' --coverage-prefix=foo --coverage-output=foo 2>&1 | grep -q 
'Gjs-WARNING.*--coverage-output'
 report "--coverage-output after script should succeed but give a warning"
 rm -f foo/coverage.lcov
 
 # --version works
-"$gjs" --version >/dev/null
+$gjs --version >/dev/null
 report "--version should work"
-test -n "$("$gjs" --version)"
+test -n "$($gjs --version)"
 report "--version should print something"
 
 # --version after a script goes to the script
 script='if(ARGV[0] !== "--version") imports.system.exit(1)'
-"$gjs" -c "$script" --version
+$gjs -c "$script" --version
 report "--version after -c should be passed to script"
-test -z "$("$gjs" -c "$script" --version)"
+test -z "$($gjs -c "$script" --version)"
 report "--version after -c should not print anything"
 
 # interpreter handles queued promise jobs correctly
-output=$("$gjs" promise.js)
+output=$($gjs promise.js)
 test $? -eq 42
 report "interpreter should exit with the correct exit code from a queued promise job"
 test -n "$output" -a -z "${output##*Should be printed*}"
@@ -158,9 +158,9 @@ report "interpreter should run queued promise jobs before finishing"
 test -n "${output##*Should not be printed*}"
 report "interpreter should stop running jobs when one calls System.exit()"
 
-"$gjs" -c "Promise.resolve().then(() => { throw new Error(); });" 2>&1 | grep -q 'Gjs-WARNING.*Unhandled 
promise rejection.*[sS]tack trace'
+$gjs -c "Promise.resolve().then(() => { throw new Error(); });" 2>&1 | grep -q 'Gjs-WARNING.*Unhandled 
promise rejection.*[sS]tack trace'
 report "unhandled promise rejection should be reported"
-test -z $("$gjs" awaitcatch.js)
+test -z $($gjs awaitcatch.js)
 report "catching an await expression should not cause unhandled rejection"
 
 rm -f exit.js help.js promise.js awaitcatch.js
diff --git a/installed-tests/scripts/testWarnings.sh b/installed-tests/scripts/testWarnings.sh
index ebf3143..804cfb6 100755
--- a/installed-tests/scripts/testWarnings.sh
+++ b/installed-tests/scripts/testWarnings.sh
@@ -1,9 +1,9 @@
 #!/bin/sh
 
 if test "$GJS_USE_UNINSTALLED_FILES" = "1"; then
-    gjs="$TOP_BUILDDIR/gjs-console"
+    gjs="$LOG_COMPILER $LOG_FLAGS $TOP_BUILDDIR/gjs-console"
 else
-    gjs="gjs-console"
+    gjs="$LOG_COMPILER $LOG_FLAGS gjs-console"
 fi
 
 total=0
@@ -18,11 +18,11 @@ report () {
     fi
 }
 
-"$gjs" -c 'imports.signals.addSignalMethods({connect: "foo"})' 2>&1 | \
+$gjs -c 'imports.signals.addSignalMethods({connect: "foo"})' 2>&1 | \
     grep -q 'addSignalMethods is replacing existing .* connect method'
 report "overwriting method with Signals.addSignalMethods() should warn"
 
-"$gjs" -c 'imports.gi.GLib.get_home_dir("foobar")' 2>&1 | \
+$gjs -c 'imports.gi.GLib.get_home_dir("foobar")' 2>&1 | \
     grep -q 'Too many arguments to .*: expected 0, got 1'
 report "passing too many arguments to a GI function should warn"
 
diff --git a/test/run-test b/test/run-test
index 028ffd3..75f62cf 100755
--- a/test/run-test
+++ b/test/run-test
@@ -6,4 +6,4 @@ if test -z "$1"; then
     exit 1
 fi
 
-"$1" --tap --keep-going --verbose
+$LOG_COMPILER $LOG_FLAGS "$1" --tap --keep-going --verbose
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]