valgrind default switched to off. 57/26757/2
authorMats Wichmann <mats@linux.com>
Sun, 12 Aug 2018 13:55:19 +0000 (07:55 -0600)
committerMats Wichmann <mats@linux.com>
Wed, 22 Aug 2018 19:12:49 +0000 (19:12 +0000)
A construction variable, VALGRIND_CHECKS, is added to the environment if
the target supports valgrind (currently that list is is linux and darwin).
It defaults to True.  If TEST mode is on, then the various unit test
locations will call the run_tests() tool which builds a command line
to run the test. If two conditions are met - (1) a pathname indicating
where to store the valgrind output is passed as the second argument
and (2) VALGRIND_CHECKS is set in the environment and evaluates True,
then that command line will be written to run the test under valgrind
control. In practical terms, This means the (linux) unit_tests builder
will run tests under valgrind. This happens on every patch submitted
to Gerrit, as part of the Jenkins CI setup.

At the moment, valgrind reports are not actively pursued by developers,
and are proving unstable (that is the terminology used by the Jenkins
valgrind report plugin itself, indicating that results differ from run
to run). Many patches have been stalled by the valgrind report plugin
marking the unit_tests build a failure, even if the patch had no
impact at all on the run.

Until this situation improves, flip the VALGRIND_CHECKS default to
False. Any developer can set up their own valgrind run at any time
by passing TEST=1 VALGRIND_CHECKS=1 to scons.

The run_test docstring is reworded.

Change-Id: I2519ab8771e2a9e299853a0e1574a10568db68a2
Signed-off-by: Mats Wichmann <mats@linux.com>
build_common/SConscript
tools/scons/RunTest.py

index d5a6eb9..b485ffc 100644 (file)
@@ -228,7 +228,7 @@ if target_os in targets_support_valgrind:
     help_vars.Add(
         BoolVariable('VALGRIND_CHECKS',
                      'Build support for running code coverage checks',
-                      default=True))
+                      default=False))
 
 targets_support_gcov = ['linux', 'darwin']
 if target_os in targets_support_gcov:
index 54f3f93..653fbbe 100644 (file)
@@ -20,19 +20,28 @@ import string
 
 def run_test(env, run_valgrind, test, test_targets=None):
     """
-    Run test using the given SCons environment.
-
-    Test results go to the test_out subdir of the build directory,
-    that path is passed to Googletest.
-
-    If run_valgrind is true, build a log filename from the test path
-    and run the test under valgrind (if the platform supports it).
-    Jenkins is configured to pick up valgrind output files ending in
-    .memcheck and include them in the build report.
-
-    Note that the test path should not include the build directory
-    where binaries are placed.  The build directory will be prepended
-    to the test path automatically.
+    Run a test using the given SCons construction environment.
+
+    If `run_valgrind` is True, build a log filename from the test path
+    and run the test under valgrind control if the platform supports it
+    and valgrind testing is enabled.  Note as an artifact of an earlier
+    scheme, most callers build the pathname and pass it as run_valgrind;
+    this is ignored and only the truth value is used.
+
+    `test` is the path relative to the top of the build directory of the
+    test binary. This function itself prepends BUILD_DIR.  Test results
+    go to BUILD_DIR/test_out - that path is passed to Googletest.
+
+    `test_targets` are targets to test as set up in the calling SConscript
+    - these are set as explicit dependencies so that the tests always
+    run rather than running only if computed dependencies have changed.
+    The default is the name 'test'
+
+    Note that because test results will be collected by Jenkins plugins
+    and reported on, the test run is configured not to "fail" in the
+    scons sense - see the hyphen added in the env.Command call. Jenkins
+    also picks up valgrind output files ending in .memcheck and include
+    them in the valgrind report.
     """
 
     build_dir = env.get('BUILD_DIR')
@@ -92,9 +101,10 @@ def run_test(env, run_valgrind, test, test_targets=None):
 
     if env.get('TARGET_OS') in ['linux']:
         env.Depends('ut' + test , os.path.join(build_dir, test))
-    # a hyphen in front of the action means ignore the result: let all tests run
-    # in a Jenkins run, results are collected at the end, so
-    # we don't have to decide anything here.
+
+    # A hyphen in front of the action means ignore the result.
+    # We want all the tests to run for Jenkins to report on the
+    # full results, don't "fail" here because a unit test did not pass.
     ut = env.Command('ut' + test, None, '-' + test_cmd)
     env.Depends(ut, test_targets)
     env.AlwaysBuild(ut)