Extend test rule to handle testscripts

4 files changed, 258 insertions, 144 deletions

diff --git a/build2/test/rule b/build2/test/rule
index 8d18a35..e6cdc37 100644
--- a/build2/test/rule
+++ b/build2/test/rule
@@ -25,6 +25,9 @@ namespace build2
       apply (action, target&, const match_result&) const;
 
       static target_state
+      perform_script (action, target&);
+
+      static target_state
       perform_test (action, target&);
     };
   }
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index 4cd91d4..c133698 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -9,6 +9,11 @@
 #include <build2/algorithm>
 #include <build2/diagnostics>
 
+#include <build2/test/target>
+
+#include <build2/test/script/parser>
+#include <build2/test/script/runner>
+
 using namespace std;
 using namespace butl;
 
@@ -16,21 +21,56 @@ namespace build2
 {
   namespace test
   {
+    struct match_data
+    {
+      bool test = false;
+      bool script = false;
+    };
+
+    static_assert (sizeof (match_data) <= target::data_size,
+                   "insufficient space");
+
     match_result rule::
     match (action a, target& t, const string&) const
     {
-      // First determine if this is a test. This is controlled by the test
-      // variable. Also, it feels redundant to specify, say, "test = true" and
-      // "test.output = test.out" -- the latter already says this is a test.
+      match_data md;
+
+      // We have two very different cases: testscript and simple test (plus it
+      // may not be a testable target at all). So as the first step determine
+      // which case this is. If we have any prerequisites of the test{} type,
+      // then this is the testscript case.
       //
-      bool r (false);
+      for (prerequisite_member p: group_prerequisite_members (a, t))
+      {
+        if (p.is_a<testscript> ())
+        {
+          md.script = true;
+          break;
+        }
+      }
+
+      if (md.script)
+      {
+        // We treat this target as testable unless the test variable is
+        // explicitly set to false.
+        //
+        lookup l (t["test"]);
+        md.test = !l || cast<bool> (l);
+      }
+      else
       {
+        // For the simple case whether this is a test is controlled by the
+        // test variable. Also, it feels redundant to specify, say, "test =
+        // true" and "test.output = test.out" -- the latter already says this
+        // is a test.
+        //
+
         // Use lookup depths to figure out who "overrides" whom.
         //
         auto p (t.find ("test"));
 
         if (p.first && cast<bool> (p.first))
-          r = true;
+          md.test = true;
         else
         {
           auto test = [&t, &p] (const char* n)
@@ -38,7 +78,8 @@ namespace build2
             return t.find (n).second < p.second;
           };
 
-          r = test ("test.input")   ||
+          md.test =
+            test ("test.input")     ||
             test ("test.output")    ||
             test ("test.roundtrip") ||
             test ("test.options")   ||
@@ -46,185 +87,251 @@ namespace build2
         }
       }
 
-      // If this is the update pre-operation, then all we really need to
-      // do is say we are not a match and the standard matching machinery
-      // will (hopefully) find the rule to update this target.
-      //
-      // There is one thing that compilates this simple approach: test
-      // input/output. While normally they will be existing (in src_base)
-      // files, they could also be auto-generated. In fact, they could
-      // only be needed for testing, which means the normall update won't
-      // even know about them (nor clean, for that matter; this is why we
-      // need cleantest).
-      //
-      // To make generated input/output work we will have to cause their
-      // update ourselves. I other words, we may have to do some actual
-      // work for (update, test), and not simply "guide" (update, 0) as
-      // to which targets need updating. For how exactly we are going to
-      // do it, see apply() below.
-      //
-      match_result mr (t, r);
+      match_result mr (t);
 
-      // If this is the update pre-operation, change the recipe action
-      // to (update, 0) (i.e., "unconditional update").
+      // If this target is testable and this is the update pre-operation, then
+      // all we really need to do is say we are not a match and the standard
+      // matching machinery will (hopefully) find the rule to update this
+      // target.
       //
-      if (r && a.operation () == update_id)
-        mr.recipe_action = action (a.meta_operation (), update_id);
+      if (md.test && a.operation () == update_id)
+      {
+        // And this is exactly what we do for the testscript case.
+        //
+        if (md.script)
+          return nullptr;
+        else
+          // For the simple case there is one thing that compilates this
+          // simple approach: test input/output. While normally they will be
+          // existing (in src_base) files, they could also be auto-generated.
+          // In fact, they could only be needed for testing, which means the
+          // normall update won't even know about them (nor clean, for that
+          // matter; this is why we need cleantest).
+          //
+          // @@ Maybe we should just say if input/output are generated, then
+          //    they must be explicitly listed as prerequisites? Then no need
+          //    for cleantest but they will be updated even when not needed.
+          //
+          // To make generated input/output work we will have to cause their
+          // update ourselves. In other words, we may have to do some actual
+          // work for (update, test), and not simply "guide" (update, 0) as to
+          // which targets need updating. For how exactly we are going to do
+          // it, see apply() below.
+          //
+          // At this stage we need to change the recipe action to (update, 0)
+          // (i.e., "unconditional update").
+          //
+          mr.recipe_action = action (a.meta_operation (), update_id);
+      }
 
+      // Note that we match even if this target is not testable so that we
+      // can ignore it (see apply()).
+      //
+      t.data (md); // Save the data in the target's auxilary storage.
       return mr;
     }
 
     recipe rule::
-    apply (action a, target& t, const match_result& mr) const
+    apply (action a, target& t, const match_result&) const
     {
       tracer trace ("test::rule::apply");
 
-      if (!mr.bvalue) // Not a test.
-        return noop_recipe;
+      match_data md (move (t.data<match_data> ()));
+      t.clear_data (); // In case delegated-to rule also uses aux storage.
 
-      // Ok, if we are here, then this means:
-      //
-      // 1. This target is a test.
-      // 2. The action is either
-      //    a. (perform, test, 0) or
-      //    b. (*, update, install)
-      //
-      // In both cases, the next step is to see if we have test.{input,
-      // output,roundtrip}.
-      //
+      if (!md.test)
+        return noop_recipe;
 
-      // We should have either arguments or input/roundtrip. Again, use
-      // lookup depth to figure out who takes precedence.
+      // If we are here, then the target is testable.
       //
-      auto ip (t.find ("test.input"));
-      auto op (t.find ("test.output"));
-      auto rp (t.find ("test.roundtrip"));
-      auto ap (t.find ("test.arguments"));
-
-      auto test = [&t] (pair<lookup, size_t>& x, const char* xn,
-                        pair<lookup, size_t>& y, const char* yn)
+      if (md.script)
       {
-        if (x.first && y.first)
+        // If we are here, then the action is (perform, test, 0).
+        //
+        // Collect all the testscript targets in prerequisite_targets.
+        //
+        for (prerequisite_member p: group_prerequisite_members (a, t))
         {
-          if (x.second == y.second)
-            fail << "both " << xn << " and " << yn << " specified for "
-                 << "target " << t;
-
-          (x.second < y.second ? y : x) = make_pair (lookup (), size_t (~0));
+          if (p.is_a<testscript> ())
+            t.prerequisite_targets.push_back (&p.search ());
         }
-      };
-
-      test (ip, "test.input",     ap, "test.arguments");
-      test (rp, "test.roundtrip", ap, "test.arguments");
-      test (ip, "test.input",     rp, "test.roundtrip");
-      test (op, "test.output",    rp, "test.roundtrip");
 
-      const name* in;
-      const name* on;
-
-      // Reduce the roundtrip case to input/output.
-      //
-      if (rp.first)
-      {
-        in = on = &cast<name> (rp.first);
+        return &perform_script;
       }
       else
       {
-        in = ip.first ? &cast<name> (ip.first) : nullptr;
-        on = op.first ? &cast<name> (op.first) : nullptr;
-      }
-
-      // Resolve them to targets, which normally would be existing files
-      // but could also be targets that need updating.
-      //
-      scope& bs (t.base_scope ());
-
-      // @@ OUT: what if this is a @-qualified pair or names?
-      //
-      target* it (in != nullptr ? &search (*in, bs) : nullptr);
-      target* ot (on != nullptr ? in == on ? it : &search (*on, bs) : nullptr);
+        // If we are here, then the action is either
+        //   a. (perform, test, 0) or
+        //   b. (*, update, 0)
+        //
+        // In both cases, the next step is to see if we have test.{input,
+        // output,roundtrip}.
+        //
 
-      if (a.operation () == update_id)
-      {
-        // First see if input/output are existing, up-to-date files. This
-        // is a common case optimization.
+        // We should have either arguments or input/roundtrip. Again, use
+        // lookup depth to figure out who takes precedence.
         //
-        if (it != nullptr)
-        {
-          build2::match (a, *it);
+        auto ip (t.find ("test.input"));
+        auto op (t.find ("test.output"));
+        auto rp (t.find ("test.roundtrip"));
+        auto ap (t.find ("test.arguments"));
 
-          if (it->state () == target_state::unchanged)
+        auto test = [&t] (pair<lookup, size_t>& x, const char* xn,
+                          pair<lookup, size_t>& y, const char* yn)
+        {
+          if (x.first && y.first)
           {
-            unmatch (a, *it);
-            it = nullptr;
+            if (x.second == y.second)
+              fail << "both " << xn << " and " << yn << " specified for "
+                   << "target " << t;
+
+            (x.second < y.second ? y : x) = make_pair (lookup (), size_t (~0));
           }
+        };
+
+        test (ip, "test.input",     ap, "test.arguments");
+        test (rp, "test.roundtrip", ap, "test.arguments");
+        test (ip, "test.input",     rp, "test.roundtrip");
+        test (op, "test.output",    rp, "test.roundtrip");
+
+        const name* in;
+        const name* on;
+
+        // Reduce the roundtrip case to input/output.
+        //
+        if (rp.first)
+        {
+          in = on = &cast<name> (rp.first);
+        }
+        else
+        {
+          in = ip.first ? &cast<name> (ip.first) : nullptr;
+          on = op.first ? &cast<name> (op.first) : nullptr;
         }
 
-        if (ot != nullptr)
+        // Resolve them to targets, which normally would be existing files
+        // but could also be targets that need updating.
+        //
+        scope& bs (t.base_scope ());
+
+        // @@ OUT: what if this is a @-qualified pair or names?
+        //
+        target* it (in != nullptr ? &search (*in, bs) : nullptr);
+        target* ot (on != nullptr
+                    ? in == on ? it : &search (*on, bs)
+                    : nullptr);
+
+        if (a.operation () == update_id)
         {
-          if (in != on)
+          // First see if input/output are existing, up-to-date files. This
+          // is a common case optimization.
+          //
+          if (it != nullptr)
           {
-            build2::match (a, *ot);
+            build2::match (a, *it);
 
-            if (ot->state () == target_state::unchanged)
+            if (it->state () == target_state::unchanged)
             {
-              unmatch (a, *ot);
-              ot = nullptr;
+              unmatch (a, *it);
+              it = nullptr;
             }
           }
-          else
-            ot = it;
-        }
 
+          if (ot != nullptr)
+          {
+            if (in != on)
+            {
+              build2::match (a, *ot);
 
-        // Find the "real" update rule, that is, the rule that would
-        // have been found if we signalled that we do not match from
-        // match() above.
-        //
-        recipe d (match_delegate (a, t).first);
+              if (ot->state () == target_state::unchanged)
+              {
+                unmatch (a, *ot);
+                ot = nullptr;
+              }
+            }
+            else
+              ot = it;
+          }
 
-        // If we have no input/output that needs updating, then simply
-        // redirect to it.
-        //
-        if (it == nullptr && ot == nullptr)
-          return d;
+          // Find the "real" update rule, that is, the rule that would have
+          // been found if we signalled that we do not match from match()
+          // above.
+          //
+          recipe d (match_delegate (a, t).first);
 
-        // Ok, time to handle the worst case scenario: we need to
-        // cause update of input/output targets and also delegate
-        // to the real update.
-        //
-        return [it, ot, dr = move (d)] (action a, target& t) -> target_state
-        {
-          // Do the general update first.
+          // If we have no input/output that needs updating, then simply
+          // redirect to it.
           //
-          target_state r (execute_delegate (dr, a, t));
+          if (it == nullptr && ot == nullptr)
+            return d;
 
-          if (it != nullptr)
-            r |= execute (a, *it);
+          // Ok, time to handle the worst case scenario: we need to cause
+          // update of input/output targets and also delegate to the real
+          // update.
+          //
+          return [it, ot, dr = move (d)] (action a, target& t) -> target_state
+          {
+            // Do the general update first.
+            //
+            target_state r (execute_delegate (dr, a, t));
 
-          if (ot != nullptr)
-            r |= execute (a, *ot);
+            if (it != nullptr)
+              r |= execute (a, *it);
 
-          return r;
-        };
+            if (ot != nullptr)
+              r |= execute (a, *ot);
+
+            return r;
+          };
+        }
+        else
+        {
+          // Cache the targets in our prerequsite targets lists where they can
+          // be found by perform_test(). If we have either or both, then the
+          // first entry is input and the second -- output (either can be
+          // NULL).
+          //
+          if (it != nullptr || ot != nullptr)
+          {
+            auto& pts (t.prerequisite_targets);
+            pts.resize (2, nullptr);
+            pts[0] = it;
+            pts[1] = ot;
+          }
+
+          return &perform_test;
+        }
       }
-      else
+    }
+
+    target_state rule::
+    perform_script (action, target& t)
+    {
+      using namespace script;
+
+      for (target* pt: t.prerequisite_targets)
       {
-        // Cache the targets in our prerequsite targets lists where they
-        // can be found by perform_test(). If we have either or both,
-        // then the first entry is input and the second -- output (either
-        // can be NULL).
-        //
-        if (it != nullptr || ot != nullptr)
+        testscript& st (*pt->is_a<testscript> ());
+        const path& sp (st.path ());
+        assert (!sp.empty ()); // Should have been assigned by update.
+
+        text << "test " << t << " with " << st;
+
+        try
         {
-          auto& pts (t.prerequisite_targets);
-          pts.resize (2, nullptr);
-          pts[0] = it;
-          pts[1] = ot;
-        }
+          ifdstream ifs (sp);
+          parser p;
+          concurrent_runner run;
 
-        return &perform_test;
+          p.parse (ifs, sp, t, st, run);
+        }
+        catch (const io_error& e)
+        {
+          fail << "unable to read testscript " << sp << ": " << e.what ();
+        }
       }
+
+      return target_state::changed;
     }
 
     // The format of args shall be:
@@ -298,7 +405,8 @@ namespace build2
     {
       // @@ Would be nice to print what signal/core was dumped.
       //
-      // @@ Doesn't have to be a file target if we have test.cmd.
+      // @@ Doesn't have to be a file target if we have test.cmd (or
+      //    just use test which is now path).
       //
 
       file& ft (static_cast<file&> (t));
diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx
index 601cbd4..f067e84 100644
--- a/build2/test/script/parser.cxx
+++ b/build2/test/script/parser.cxx
@@ -197,7 +197,7 @@ namespace build2
 
         // Make sure we don't have any pending positions to fill.
         //
-        auto check_pending = [p, this] (const location& l)
+        auto check_pending = [&p, this] (const location& l)
         {
           const char* what (nullptr);
 
@@ -448,7 +448,7 @@ namespace build2
           case type::newline:
             {
               done = true;
-              break;
+              continue;
             }
           default:
             {
@@ -520,6 +520,8 @@ namespace build2
 
           expire_mode ();
         }
+        else
+          next (t, tt);
 
         // Now that we have all the pieces, run the test.
         //
diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 3789281..69b0213 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -13,9 +13,10 @@ namespace build2
     namespace script
     {
       void concurrent_runner::
-      run (const test&)
+      run (const test& t)
       {
-        assert (false); // @@ TODO
+        // @@ TODO
+        text << "run " << t.program;
       }
     }
   }