Further test module development

7 files changed, 305 insertions, 55 deletions

diff --git a/build/algorithm b/build/algorithm
index f108a8a..d01614c 100644
--- a/build/algorithm
+++ b/build/algorithm
@@ -44,7 +44,7 @@ namespace build
           const dir_path& dir,
           const std::string& name,
           const std::string* ext,
-          scope* scope);
+          scope*);
 
   // As above but specify the target type as template argument.
   //
@@ -53,7 +53,15 @@ namespace build
   search (const dir_path& dir,
           const std::string& name,
           const std::string* ext,
-          scope* scope);
+          scope*);
+
+  // Search for a target identified by the name. The semantics
+  // is "as if" we first created a prerequisite based on this
+  // name in exactly the same way as the parser would and then
+  // searched based on this prerequisite.
+  //
+  target&
+  search (name, scope&);
 
   // Match a rule to the action/target with ambiguity detection.
   //
diff --git a/build/algorithm.cxx b/build/algorithm.cxx
index a19db36..8893045 100644
--- a/build/algorithm.cxx
+++ b/build/algorithm.cxx
@@ -41,6 +41,20 @@ namespace build
     return create_new_target (pk);
   }
 
+  target&
+  search (name n, scope& s)
+  {
+    const string* e;
+    const target_type* tt (s.find_target_type (n, e));
+
+    if (tt == nullptr)
+      fail << "unknown target type " << n.type << " in name " << n;
+
+    n.dir.normalize ();
+
+    return search (*tt, move (n.dir), move (n.value), e, &s);
+  }
+
   match_result_impl
   match_impl (action a, target& t, bool apply)
   {
diff --git a/build/diagnostics b/build/diagnostics
index 2eaaa84..b8b1090 100644
--- a/build/diagnostics
+++ b/build/diagnostics
@@ -24,6 +24,8 @@ namespace build
   using butl::path;
   using butl::dir_path;
 
+  struct diag_record;
+
   // Throw this exception to terminate the build. The handler should
   // assume that the diagnostics has already been issued.
   //
@@ -78,9 +80,18 @@ namespace build
   // nameN arg arg ... nullptr nullptr
   //
   void
+  print_process (diag_record&, const char* const* args, std::size_t n = 0);
+
+  void
   print_process (const char* const* args, std::size_t n = 0);
 
   inline void
+  print_process (diag_record& dr, const cstrings& args)
+  {
+    print_process (dr, args.data (), args.size ());
+  }
+
+  inline void
   print_process (const cstrings& args)
   {
     print_process (args.data (), args.size ());
@@ -112,8 +123,6 @@ namespace build
   template <typename> struct diag_prologue;
   template <typename> struct diag_mark;
 
-  struct diag_record;
-
   typedef void (*diag_epilogue) (const diag_record&);
 
   struct diag_record
diff --git a/build/diagnostics.cxx b/build/diagnostics.cxx
index f12119c..a254860 100644
--- a/build/diagnostics.cxx
+++ b/build/diagnostics.cxx
@@ -160,7 +160,12 @@ namespace build
   print_process (const char* const* args, size_t n)
   {
     diag_record r (text);
+    print_process (r, args, n);
+  }
 
+  void
+  print_process (diag_record& r, const char* const* args, size_t n)
+  {
     size_t m (0);
     const char* const* p (args);
     do
diff --git a/build/target.cxx b/build/target.cxx
index 5aa1214..f6fc5c8 100644
--- a/build/target.cxx
+++ b/build/target.cxx
@@ -356,13 +356,14 @@ namespace build
                      (e != nullptr ? e : &extension_pool.find ("")));
   }
 
+  constexpr const char file_ext[] = "";
   const target_type file::static_type
   {
     typeid (file),
     "file",
     &path_target::static_type,
     &file_factory,
-    nullptr, // Factory always assigns an extension.
+    &target_extension_fix<file_ext>,
     &search_file,
     false
   };
diff --git a/build/test/rule.cxx b/build/test/rule.cxx
index 91ddb4f..b8d7428 100644
--- a/build/test/rule.cxx
+++ b/build/test/rule.cxx
@@ -22,14 +22,56 @@ namespace build
     match_result rule::
     match (action a, target& t, const std::string&) const
     {
-      // First determine if this is a test.
+      // First determine if this is a test. This is controlled by
+      // the test target variable and text.<tt> scope variables.
+      // Also, it feels redundant to specify, say, "test = true"
+      // and "test.output = test.out" -- the latter already says
+      // this is a test. So take care of that as well.
       //
-      auto v (t.vars["test"]);
+      bool r (false);
+      value_proxy v;
 
-      if (!v)
-        v.rebind (t.base_scope ()[string("test.") + t.type ().name]);
+      for (auto p (t.vars.find_namespace ("test"));
+           p.first != p.second;
+           ++p.first)
+      {
+        const variable& var (p.first->first);
+        value_ptr& val (p.first->second);
+
+        // If we have test, then always use that.
+        //
+        if (var.name == "test")
+        {
+          v.rebind (value_proxy (val, t));
+          break;
+        }
+
+        // Otherwise check for variables that would indicate this
+        // is a test.
+        //
+        if (var.name == "test.input"     ||
+            var.name == "test.ouput"     ||
+            var.name == "test.roundtrip" ||
+            var.name == "test.options"   ||
+            var.name == "test.arguments")
+        {
+          r = true;
+          break;
+        }
+      }
+
+      if (!r)
+      {
+        // See if the is a scope variable.
+        //
+        if (!v)
+          v.rebind (t.base_scope ()[string("test.") + t.type ().name]);
+
+        r = v && v.as<bool> ();
 
-      if (!v || !v.as<bool> ())
+      }
+
+      if (!r)
         return match_result (t, false); // "Not a test" result.
 
       // If this is the update pre-operation, make someone else do
@@ -42,14 +84,141 @@ namespace build
     }
 
     recipe rule::
-    apply (action a, target&, const match_result& mr) const
+    apply (action a, target& t, const match_result& mr) const
     {
+      tracer trace ("test::rule::apply");
+
       if (!mr.value) // Not a test.
         return noop_recipe;
 
-      return a == action (perform_id, test_id)
-        ? &perform_test
-        : noop_recipe; // Don't do anything for other meta-operations.
+      // Don't do anything for other meta-operations.
+      //
+      if (a != action (perform_id, test_id))
+        return noop_recipe;
+
+      // See if we have test.{input,output,roundtrip}. First check the
+      // target-specific vars since they override any scope ones.
+      //
+      auto iv (t.vars["test.input"]);
+      auto ov (t.vars["test.output"]);
+      auto rv (t.vars["test.roundtrip"]);
+
+      // Can either be input or arguments.
+      //
+      auto av (t.vars["test.arguments"]);
+
+      if (av)
+      {
+        if (iv)
+          fail << "both test.input and test.arguments specified for "
+               << "target " << t;
+
+        if (rv)
+          fail << "both test.roundtrip and test.arguments specified for "
+               << "target " << t;
+      }
+
+      scope& bs (t.base_scope ());
+
+      if (!iv && !ov && !rv)
+      {
+        string n ("test.");
+        n += t.type ().name;
+
+        const variable& in (variable_pool.find (n + ".input"));
+        const variable& on (variable_pool.find (n + ".output"));
+        const variable& rn (variable_pool.find (n + ".roundtrip"));
+
+        // We should only keep value(s) that were specified together
+        // in the innermost scope.
+        //
+        for (scope* s (&bs); s != nullptr; s = s->parent_scope ())
+        {
+          ov.rebind (s->vars[on]);
+
+          if (!av) // Not overriden at target level by test.arguments?
+          {
+            iv.rebind (s->vars[in]);
+            rv.rebind (s->vars[rn]);
+          }
+
+          if (iv || ov || rv)
+            break;
+        }
+      }
+
+      const name* i;
+      const name* o;
+
+      // Reduce the roundtrip case to input/output.
+      //
+      if (rv)
+      {
+        if (iv || ov)
+          fail << "both test.roundtrip and test.input/output specified "
+               << "for target " << t;
+
+        i = o = rv.as<const name*> ();
+      }
+      else
+      {
+        i = iv ? iv.as<const name*> () : nullptr;
+        o = ov ? ov.as<const name*> () : nullptr;
+      }
+
+      // Resolve them to targets (normally just files) and cache 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).
+      //
+      auto& pts (t.prerequisite_targets);
+
+      if (i != nullptr || o != nullptr)
+        pts.resize (2, nullptr);
+
+      //@@ We should match() them, but for update, not test.
+      //@@ If not doing this for some reason, need to then verify
+      //   path was assigned (i.e., matched an existing file).
+      //
+      if (i != nullptr)
+        pts[0] = &search (*i, bs);
+
+      if (o != nullptr)
+        pts[1] = i == o ? pts[0] : &search (*o, bs);
+
+      return &perform_test;
+    }
+
+    static void
+    add_arguments (cstrings& args, target& t, const char* n)
+    {
+      string var ("test.");
+      var += n;
+
+      auto v (t.vars[var]);
+
+      if (!v)
+      {
+        var.resize (5);
+        var += t.type ().name;
+        var += '.';
+        var += n;
+        v.rebind (t.base_scope ()[var]);
+      }
+
+      if (v)
+      {
+        for (const name& n: v.as<const list_value&> ())
+        {
+          if (n.simple ())
+            args.push_back (n.value.c_str ());
+          else if (n.directory ())
+            args.push_back (n.dir.string ().c_str ());
+          else
+            fail << "expected argument instead of " << n <<
+              info << "in variable " << var;
+        }
+      }
     }
 
     // The format of args shall be:
@@ -60,7 +229,10 @@ namespace build
     // nameN arg arg ... nullptr nullptr
     //
     static bool
-    pipe_process (char const** args, process* prev = nullptr)
+    run_test (target& t,
+              diag_record& dr,
+              char const** args,
+              process* prev = nullptr)
     {
       // Find the next process, if any.
       //
@@ -74,54 +246,94 @@ namespace build
       int out (*next == nullptr ? 2 : -1);
       bool pr, wr;
 
-      if (prev == nullptr)
+      try
       {
-        // First process.
-        //
-        process p (args, 0, out);
-        pr = *next == nullptr || pipe_process (next, &p);
-        wr = p.wait ();
+        if (prev == nullptr)
+        {
+          // First process.
+          //
+          process p (args, 0, out);
+          pr = *next == nullptr || run_test (t, dr, next, &p);
+          wr = p.wait ();
+        }
+        else
+        {
+          // Next process.
+          //
+          process p (args, *prev, out);
+          pr = *next == nullptr || run_test (t, dr, next, &p);
+          wr = p.wait ();
+        }
       }
-      else
+      catch (const process_error& e)
       {
-        // Next process.
-        //
-        process p (args, *prev, out);
-        pr = *next == nullptr || pipe_process (next, &p);
-        wr = p.wait ();
+        error << "unable to execute " << args[0] << ": " << e.what ();
+
+        if (e.child ())
+          exit (1);
+
+        throw failed ();
       }
 
       if (!wr)
       {
-        // @@ Needs to go into the same diag record.
-        //
-        error << "non-zero exit status from:";
-        print_process (args);
+        if (pr) // First failure?
+          dr << fail << "test " << t << " failed"; // Multi test: test 1.
+
+        dr << error << "non-zero exit status: ";
+        print_process (dr, args);
       }
 
       return pr && wr;
     }
 
-
     target_state rule::
     perform_test (action, target& t)
     {
       // @@ Would be nice to print what signal/core was dumped.
       //
-
       // @@ Doesn't have to be a file target if we have test.cmd.
       //
+
       file& ft (static_cast<file&> (t));
       assert (!ft.path ().empty ()); // Should have been assigned by update.
 
-      cstrings args {ft.path ().string ().c_str (), nullptr};
+      cstrings args {ft.path ().string ().c_str ()};
+
+      // Do we have options?
+      //
+      add_arguments (args, t, "options");
+
+      // Do we have input?
+      //
+      auto& pts (t.prerequisite_targets);
+      if (pts.size () != 0 && pts[0] != nullptr)
+      {
+        file& it (static_cast<file&> (*pts[0]));
+        assert (!it.path ().empty ()); // Should have been assigned.
+        args.push_back (it.path ().string ().c_str ());
+      }
+      // Maybe arguments then?
+      //
+      else
+        add_arguments (args, t, "arguments");
 
-      args.push_back ("diff");
-      args.push_back ("-u");
-      args.push_back ("test.std");
-      args.push_back ("-");
       args.push_back (nullptr);
 
+      // Do we have output?
+      //
+      if (pts.size () != 0 && pts[1] != nullptr)
+      {
+        file& ot (static_cast<file&> (*pts[1]));
+        assert (!ot.path ().empty ()); // Should have been assigned.
+
+        args.push_back ("diff");
+        args.push_back ("-u");
+        args.push_back (ot.path ().string ().c_str ());
+        args.push_back ("-");
+        args.push_back (nullptr);
+      }
+
       args.push_back (nullptr); // Second.
 
       if (verb)
@@ -129,28 +341,17 @@ namespace build
       else
         text << "test " << t;
 
-      try
       {
-        if (!pipe_process (args.data ()))
+        diag_record dr;
+
+        if (!run_test (t, dr, args.data ()))
         {
-          //@@ Need to use the same diag record.
-          //
-          error << "failed test:";
-          print_process (args);
-          throw failed ();
+          dr << info << "test command line: ";
+          print_process (dr, args);
         }
-
-        return target_state::changed;
       }
-      catch (const process_error& e)
-      {
-        error << "unable to execute " << args[0] << ": " << e.what ();
 
-        if (e.child ())
-          exit (1);
-
-        throw failed ();
-      }
+      return target_state::changed;
     }
   }
 }
diff --git a/build/variable b/build/variable
index 9009080..368b2f8 100644
--- a/build/variable
+++ b/build/variable
@@ -175,6 +175,9 @@ namespace build
     value_proxy (): map (nullptr), p (nullptr) {}
     value_proxy (value_ptr* p, const variable_map* m): map (m), p (p) {}
 
+    template <typename T>
+    value_proxy (value_ptr& p, const T& x): map (&x.vars), p (&p) {}
+
     void
     rebind (const value_proxy& x) {map = x.map; p = x.p;}
 
@@ -204,6 +207,15 @@ namespace build
   as<const list_value&> () const {return as<list_value&> ();}
 
   template <>
+  inline const name* value_proxy::
+  as<const name*> () const
+  {
+    const auto& lv (as<const list_value&> ());
+    assert (lv.size () < 2);
+    return lv.empty () ? nullptr : &lv.front ();
+  }
+
+  template <>
   std::string& value_proxy::
   as<std::string&> () const;