Add support for config.test.runner

15 files changed, 377 insertions, 18 deletions

diff --git a/doc/manual.cli b/doc/manual.cli
index a9d75d6..d7cf92f 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -5647,6 +5647,38 @@ forcibly causing the entire \c{test} operation to fail. See also the
 \l{testscript#builtins-timeout \c{timeout}} builtin for specifying timeouts
 from within the tests and test groups.
 
+The programs being tested can be executed via a \i{runner program} by
+specifying the \c{config.test.runner} variable. Its value has the \c{<path>
+[<options>]} form. For example:
+
+\
+b test config.test.runner=\"valgrind -q\"
+\
+
+When the runner program is specified, commands of simple and Testscript tests
+are automatically adjusted so that the runner program is executed instead,
+with the test command passed to it as arguments. For ad hoc test recipes,
+the runner program has to be handled explicitly. Specifically, if
+\c{config.test.runner} is specified, the \c{test.runner.path} and
+\c{test.runner.options} variables contain the runner program path and options,
+respectively, and are set to \c{null} otherwise. These variables can be used
+by ad hoc recipes to detect the presence of the runner program and, if so,
+arrange appropriate execution of desired commands. For example:
+
+\
+exe{hello}:
+% test
+{{
+  diag test $>
+
+  cmd = ($test.runner.path == [null] \
+    ? $> \
+    : $test.runner.path $test.runner.options $path($>))
+
+  $cmd 'World' >>>?'Hello, World!'
+}}
+\
+
 
 \h1#module-install|\c{install} Module|
 
diff --git a/doc/testscript.cli b/doc/testscript.cli
index c295a31..badbe97 100644
--- a/doc/testscript.cli
+++ b/doc/testscript.cli
@@ -1041,13 +1041,21 @@ basics/fox/baz
 \
 
 To only run individual tests, test groups, or testscript files we can specify
-their id paths in the \c{config.testscript} variable, for example:
+their id paths in the \c{config.test} variable, for example:
 
 \
 $ b test config.test=basics                      # All in basics.testscript
 $ b test config.test=basics/fox                  # All in fox
 $ b test config.test=basics/foo                  # Only foo
-$ b test 'config.test=basics/foo basics/fox/bar' # Only foo and bar
+$ b test config.test=\"basics/foo basics/fox/bar\" # Only foo and bar
+\
+
+The test commands (\c{$0}, \c{$*}) can be executed via a \i{runner program} by
+specifying the \c{config.test.runner} variable (see \l{build2#module-test
+\c{test}} module for details). For example:
+
+\
+$ b test config.test.runner=\"valgrind -q\"
 \
 
 The script working directory may exist before the execution (for example,
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 9021da1..04a257f 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -321,6 +321,16 @@ namespace build2
         parse_here_documents (t, tt, p);
         assert (tt == type::newline);
 
+        // @@ Note that currently running programs via a runner (e.g., see
+        //    test.runner) needs to be handled explicitly in ad hoc recipes.
+        //    We could potentially run them via the runner implicitly, similar
+        //    to how we do in the testscript. We would need then to match the
+        //    command program path against the recipe target ad hoc member
+        //    paths (test programs), to detect if it must be run via the
+        //    runner. The runner path/options would need to be optionally
+        //    passed to the environment constructor, similar to passing the
+        //    script deadline.
+        //
         return move (p.first);
       }
 
diff --git a/libbuild2/test/common.hxx b/libbuild2/test/common.hxx
index a43b2b1..958b541 100644
--- a/libbuild2/test/common.hxx
+++ b/libbuild2/test/common.hxx
@@ -21,11 +21,15 @@ namespace build2
       const variable& config_test;
       const variable& config_test_output;
       const variable& config_test_timeout;
+      const variable& config_test_runner;
 
       const variable& var_test;
       const variable& test_options;
       const variable& test_arguments;
 
+      const variable& test_runner_path;
+      const variable& test_runner_options;
+
       const variable& test_stdin;
       const variable& test_stdout;
       const variable& test_roundtrip;
@@ -46,6 +50,12 @@ namespace build2
       optional<duration> operation_timeout;
       optional<duration> test_timeout;
 
+      // The test.runner.{path,options} values extracted from the
+      // config.test.runner value, if any.
+      //
+      const process_path* runner_path = nullptr;
+      const strings* runner_options = nullptr;
+
       // The config.test query interface.
       //
       const names* test_ = nullptr; // The config.test value if any.
diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx
index 47a73ba..dadbd20 100644
--- a/libbuild2/test/init.cxx
+++ b/libbuild2/test/init.cxx
@@ -62,6 +62,10 @@ namespace build2
         //
         vp.insert<string> ("config.test.timeout"),
 
+        // Test command runner path and options (see the manual for semantics).
+        //
+        vp.insert<strings> ("config.test.runner"),
+
         // The test variable is a name which can be a path (with the
         // true/false special values) or a target name.
         //
@@ -69,6 +73,12 @@ namespace build2
         vp.insert<strings> ("test.options"),
         vp.insert<strings> ("test.arguments"),
 
+        // Test command runner path and options extracted from
+        // config.test.runner.
+        //
+        vp.insert<process_path> ("test.runner.path"),
+        vp.insert<strings>      ("test.runner.options"),
+
         // Prerequisite-specific.
         //
         // test.stdin and test.stdout can be used to mark a prerequisite as a
@@ -223,6 +233,46 @@ namespace build2
           m.operation_timeout = parse_timeout (t, ot);
       }
 
+      // config.test.runner
+      //
+      {
+        value& pv (rs.assign (m.test_runner_path));
+        value& ov (rs.assign (m.test_runner_options));
+
+        if (lookup l = lookup_config (rs, m.config_test_runner))
+        {
+          const strings& args (cast<strings> (l));
+
+          // Extract the runner process path.
+          //
+          {
+            const string& s (args.empty () ? empty_string : args.front ());
+
+            path p;
+            try { p = path (s); } catch (const invalid_path&) {}
+
+            if (p.empty ())
+              fail << "invalid runner path '" << s << "' in "
+                   << m.config_test_runner;
+
+            pv = run_search (p, false /* init */);
+            m.runner_path = &pv.as<process_path> ();
+          }
+
+          // Extract the runner options.
+          //
+          {
+            ov = strings (++args.begin (), args.end ());
+            m.runner_options = &ov.as<strings> ();
+          }
+        }
+        else
+        {
+          pv = nullptr;
+          ov = nullptr;
+        }
+      }
+
       //@@ TODO: Need ability to specify extra diff options (e.g.,
       //   --strip-trailing-cr, now hardcoded).
       //
diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx
index d720b25..06fb12f 100644
--- a/libbuild2/test/rule.cxx
+++ b/libbuild2/test/rule.cxx
@@ -937,12 +937,32 @@ namespace build2
         args.push_back (nullptr);
       }
 
-      // If dry-run, the target may not exist.
+      process_path pp;
+
+      // Do we have a test runner?
       //
-      process_path pp (!ctx.dry_run
-                       ? run_search     (p, true /* init */)
-                       : run_try_search (p, true));
-      args.push_back (pp.empty () ? p.string ().c_str () : pp.recall_string ());
+      if (runner_path == nullptr)
+      {
+        // If dry-run, the target may not exist.
+        //
+        pp = process_path (!ctx.dry_run
+                           ? run_search     (p, true /* init */)
+                           : run_try_search (p, true));
+
+        args.push_back (pp.empty ()
+                        ? p.string ().c_str ()
+                        : pp.recall_string ());
+      }
+      else
+      {
+        args.push_back (runner_path->recall_string ());
+
+        append_options (args, *runner_options);
+
+        // Leave it to the runner to resolve the test program path.
+        //
+        args.push_back (p.string ().c_str ());
+      }
 
       // Do we have options and/or arguments?
       //
diff --git a/libbuild2/test/script/parser.cxx b/libbuild2/test/script/parser.cxx
index 944e1c8..8179058 100644
--- a/libbuild2/test/script/parser.cxx
+++ b/libbuild2/test/script/parser.cxx
@@ -1280,7 +1280,45 @@ namespace build2
         parse_here_documents (t, tt, p);
         assert (tt == type::newline);
 
-        return move (p.first);
+        command_expr r (move (p.first));
+
+        // If the test program runner is specified, then adjust the
+        // expressions to run test programs via this runner.
+        //
+        pair<const process_path*, const strings*> tr (
+          runner_->test_runner ());
+
+        if (tr.first != nullptr)
+        {
+          for (expr_term& t: r)
+          {
+            for (command& c: t.pipe)
+            {
+              if (scope_->test_program (c.program.recall))
+              {
+                // Append the runner options and the test program path to the
+                // the arguments list and rotate the list to the left, so that
+                // it starts from the runner options. This should probably be
+                // not less efficient than inserting the program path and then
+                // the runner options at the beginning of the list.
+                //
+                strings& args (c.arguments);
+                size_t n (args.size ());
+
+                args.insert (args.end (),
+                             tr.second->begin (), tr.second->end ());
+
+                args.push_back (c.program.recall.string ());
+
+                rotate (args.begin (), args.begin () + n, args.end ());
+
+                c.program = process_path (*tr.first, false /* init */);
+              }
+            }
+          }
+        }
+
+        return r;
       }
 
       //
diff --git a/libbuild2/test/script/parser.test.cxx b/libbuild2/test/script/parser.test.cxx
index 69583ae..dbdeb57 100644
--- a/libbuild2/test/script/parser.test.cxx
+++ b/libbuild2/test/script/parser.test.cxx
@@ -39,6 +39,12 @@ namespace build2
           return true;
         }
 
+        virtual pair<const process_path*, const strings*>
+        test_runner () override
+        {
+          return make_pair (nullptr, nullptr);
+        }
+
         virtual void
         enter (scope& s, const location&) override
         {
diff --git a/libbuild2/test/script/runner.cxx b/libbuild2/test/script/runner.cxx
index 03a1f0e..eb30e02 100644
--- a/libbuild2/test/script/runner.cxx
+++ b/libbuild2/test/script/runner.cxx
@@ -21,6 +21,12 @@ namespace build2
         return common_.test (s.root.test_target, s.id_path);
       }
 
+      pair<const process_path*, const strings*> default_runner::
+      test_runner ()
+      {
+        return make_pair (common_.runner_path, common_.runner_options);
+      }
+
       void default_runner::
       enter (scope& sp, const location&)
       {
diff --git a/libbuild2/test/script/runner.hxx b/libbuild2/test/script/runner.hxx
index 22cae4e..b6a038d 100644
--- a/libbuild2/test/script/runner.hxx
+++ b/libbuild2/test/script/runner.hxx
@@ -29,6 +29,12 @@ namespace build2
         virtual bool
         test (scope&) const = 0;
 
+        // Return the runner program path and options if the test commands
+        // must be run via the runner and the pair of NULLs otherwise.
+        //
+        virtual pair<const process_path*, const strings*>
+        test_runner () = 0;
+
         // Location is the scope start location (for diagnostics, etc).
         //
         virtual void
@@ -66,6 +72,12 @@ namespace build2
         virtual bool
         test (scope& s) const override;
 
+        // Return the test.runner.{path,options} values, if config.test.runner
+        // is specified.
+        //
+        virtual pair<const process_path*, const strings*>
+        test_runner () override;
+
         virtual void
         enter (scope&, const location&) override;
 
diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx
index 3f615ee..cb367bc 100644
--- a/libbuild2/test/script/script.cxx
+++ b/libbuild2/test/script/script.cxx
@@ -94,12 +94,37 @@ namespace build2
           const_cast<path&> (id_path) = path (move (s));
         }
 
-        // Calculate the working directory path unless this is the root scope
-        // (handled in an ad hoc way).
+        // Calculate the working directory path and the test programs set
+        // unless this is the root scope (handled in an ad hoc way).
         //
         if (p != nullptr)
+        {
           const_cast<dir_path&> (*work_dir.path) =
             dir_path (*p->work_dir.path) /= id;
+
+          // Note that we could probably keep the test programs sets fully
+          // independent across the scopes and check if the program is a test
+          // by traversing the scopes upwards recursively. Note though, that
+          // the parent scope's set cannot change during the nested scope
+          // lifetime and normally contains just a single entry. Thus, it
+          // seems more efficient to get rid of the recursion by copying the
+          // set from the parent now and potentially changing it later on the
+          // test variable assignment, etc.
+          //
+          test_programs_ = p->test_programs_;
+        }
+      }
+
+      bool scope::
+      test_program (const path& p)
+      {
+        assert (!test_programs_.empty ());
+
+        return find_if (test_programs_.begin (), test_programs_.end (),
+                        [&p] (const path* tp)
+                        {
+                          return tp != nullptr ? *tp == p : false;
+                        }) != test_programs_.end ();
       }
 
       void scope::
@@ -287,6 +312,12 @@ namespace build2
           }
         }
 
+        // Reserve the entry for the test program specified via the test
+        // variable. Note that the value will be assigned by the below
+        // reset_special() call.
+        //
+        test_programs_.push_back (nullptr);
+
         // Set the special $*, $N variables.
         //
         reset_special ();
@@ -378,7 +409,8 @@ namespace build2
       void scope::
       reset_special ()
       {
-        // First assemble the $* value.
+        // First assemble the $* value and save the test variable value into
+        // the test program set.
         //
         strings s;
 
@@ -387,14 +419,24 @@ namespace build2
           s.insert (s.end (), v.begin (), v.end ());
         };
 
+        // If the test variable can't be looked up for any reason (is NULL,
+        // etc), then keep $* empty.
+        //
         if (auto l = lookup (root.test_var))
-          s.push_back (cast<path> (l).representation ());
+        {
+          const path& p (cast<path> (l));
+          s.push_back (p.representation ());
 
-        if (auto l = lookup (root.options_var))
-          append (cast<strings> (l));
+          test_programs_[0] = &p;
 
-        if (auto l = lookup (root.arguments_var))
-          append (cast<strings> (l));
+          if (auto l = lookup (root.options_var))
+            append (cast<strings> (l));
+
+          if (auto l = lookup (root.arguments_var))
+            append (cast<strings> (l));
+        }
+        else
+          test_programs_[0] = nullptr;
 
         // Keep redirects/cleanups out of $N.
         //
diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx
index 2789cab..d387a11 100644
--- a/libbuild2/test/script/script.hxx
+++ b/libbuild2/test/script/script.hxx
@@ -28,6 +28,8 @@ namespace build2
       using build2::script::redirect_type;
       using build2::script::line_type;
       using build2::script::command_expr;
+      using build2::script::expr_term;
+      using build2::script::command;
       using build2::script::deadline;
       using build2::script::timeout;
 
@@ -105,6 +107,15 @@ namespace build2
         virtual void
         create_temp_dir () override {assert (false);};
 
+        // Return true if this is a test program path.
+        //
+        // Note that currently the test program is only specified via the test
+        // variable ($0 effectively). In the future we may invent some other
+        // means of marking a program as a test (builtin, etc).
+        //
+        bool
+        test_program (const path&);
+
         // Variables.
         //
       public:
@@ -163,6 +174,15 @@ namespace build2
         location end_loc_;
 
         optional<line> if_cond_;
+
+        // Test program paths.
+        //
+        // Currently always contains a single element (see test_program() for
+        // details). While in the future there can be more of them, the zero
+        // index will always refer to the test variable value and can
+        // potentially be NULL (see reset_special() for details).
+        //
+        small_vector<const path*, 1> test_programs_;
       };
 
       // group
diff --git a/tests/recipe/buildscript/testscript b/tests/recipe/buildscript/testscript
index 2603f62..6bdbd32 100644
--- a/tests/recipe/buildscript/testscript
+++ b/tests/recipe/buildscript/testscript
@@ -406,7 +406,7 @@
       }}
       % [diag=test] test
       {{
-        cat <$path($<) >?$path($>)
+        cat <$path($>) >?$path($<)
       }}
       EOI
 
@@ -431,7 +431,7 @@
       % [diag=test] test
       {{
         depdb clear
-        cat <$path($<) >?$path($>)
+        cat <$path($>) >?$path($<)
       }}
       EOI
 
@@ -439,6 +439,53 @@
       buildfile:7:3: error: 'depdb' builtin cannot be used to perform test
       EOE
   }
+
+  : runner
+  :
+  if ($cxx.target.class != 'windows')
+  {
+    echo 'bar' >=bar;
+
+    cat <<EOI >=run;
+      #!/bin/sh
+      if test "$1" = "--trace"; then
+        shift
+        echo "$*"
+      fi
+      "$@"
+      EOI
+
+    chmod u+x run;
+
+    cat <<EOI >=buildfile;
+      foo: bar
+      {{
+        cp $path($<) $path($>)
+      }}
+      % [diag=test] test
+      {{
+        if ($test.runner.path != [null])
+          $test.runner.path $test.runner.options cat <$path($>)
+        else
+          cat <$path($>)
+        end
+      }}
+      EOI
+
+    $* test 2>>EOE;
+      cp file{foo}
+      test file{foo}
+      bar
+      EOE
+
+    $* test config.test.runner="./run --trace" 2>>EOE;
+      test file{foo}
+      cat
+      bar
+      EOE
+
+    $* clean 2>-
+  }
 }
 
 : diff-label
diff --git a/tests/test/script/runner/test-runner.testscript b/tests/test/script/runner/test-runner.testscript
new file mode 100644
index 0000000..772fa1b
--- /dev/null
+++ b/tests/test/script/runner/test-runner.testscript
@@ -0,0 +1,31 @@
+# file      : tests/test/script/runner/test-runner.testscript
+# license   : MIT; see accompanying LICENSE file
+
+.include ../common.testscript
+
++if ($cxx.target.class == 'windows')
+  exit
+end
+
++cat <<EOI >=run
+  #!/bin/sh
+  if test "$1" = "--trace"; then
+    shift
+    echo "$*"
+  fi
+  "$@"
+  EOI
+
++chmod u+x run
+
+run=$~/run
+
+: basic
+:
+$c <<"EOI" && $b "config.test.runner=$run --trace"
+  cat <'text' >'text';   # Non-test program.
+  \$* -o 'text' >>~%EOO%  # Test program.
+    %.+/driver -o text%
+    text
+    EOO
+  EOI
diff --git a/tests/test/simple/generated/testscript b/tests/test/simple/generated/testscript
index f6a89d8..9ce40ba 100644
--- a/tests/test/simple/generated/testscript
+++ b/tests/test/simple/generated/testscript
@@ -99,3 +99,30 @@ EOI
     EOE
   }
 }
+
+: runner
+:
+if ($cxx.target.class != 'windows')
+{
+  cat <<EOI >=run;
+    #!/bin/sh
+    if test "$1" = "--trace"; then
+      shift
+      echo "tracing"
+    fi
+    "$@"
+    EOI
+
+  chmod u+x run;
+
+  echo 'tracing'          >=output.in;
+  cat $src_base/output.in >+output.in;
+
+  $* config.test.runner="./run --trace" <<EOI
+  driver = $src_root/../exe{driver}
+  ./: test = $driver
+  ./: $driver
+  ./: file{output}: test.stdout = true
+  file{output}: in{output} $src_root/manifest #@@ in module
+  EOI
+}