Add support for passing target name to testscript via test variable

Such a targets is automatically resolved and converted to path.

4 files changed, 85 insertions, 27 deletions

diff --git a/build2/test/init.cxx b/build2/test/init.cxx
index bb85464..098718b 100644
--- a/build2/test/init.cxx
+++ b/build2/test/init.cxx
@@ -39,10 +39,12 @@ namespace build2
       {
         auto& v (var_pool);
 
-        // Note: none are overridable. The test variable is a path with the
-        // true/false special values.
+        // Note: none are overridable.
         //
-        v.insert<path>    ("test",           variable_visibility::target);
+        // The test variable is a name which can be a path (with the
+        // true/false special values) or a target name.
+        //
+        v.insert<name>    ("test",           variable_visibility::target);
         v.insert<name>    ("test.input",     variable_visibility::project);
         v.insert<name>    ("test.output",    variable_visibility::project);
         v.insert<name>    ("test.roundtrip", variable_visibility::project);
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index 0abc233..a8adb9e 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -68,8 +68,8 @@ namespace build2
           // We treat this target as testable unless the test variable is
           // explicitly set to false.
           //
-          lookup l (t["test"]);
-          md.test = !l || cast<path> (l).string () != "false";
+          const name* n (cast_null<name> (t["test"]));
+          md.test = n == nullptr || !n->simple () || n->value != "false";
           break;
         }
       }
@@ -88,14 +88,15 @@ namespace build2
         // Use lookup depths to figure out who "overrides" whom.
         //
         auto p (t.find ("test"));
+        const name* n (cast_null<name> (p.first));
 
-        if (p.first && cast<path> (p.first).string () != "false")
+        if (n != nullptr && n->simple () && n->value != "false")
           md.test = true;
         else
         {
-          auto test = [&t, &p] (const char* n)
+          auto test = [&t, &p] (const char* var)
           {
-            return t.find (n).second < p.second;
+            return t.find (var).second < p.second;
           };
 
           md.test =
diff --git a/build2/test/script/script b/build2/test/script/script
index d68a684..714e2c5 100644
--- a/build2/test/script/script
+++ b/build2/test/script/script
@@ -283,10 +283,12 @@ namespace build2
         lookup
         find (const variable&) const;
 
-        // As above but only look for buildfile variables.
+        // As above but only look for buildfile variables. If target_only is
+        // false then also look in scopes of the test target (this should only
+        // be done if the variable's visibility is target).
         //
         lookup
-        find_in_buildfile (const string&) const;
+        find_in_buildfile (const string&, bool target_only = true) const;
 
         // Return a value suitable for assignment. If the variable does not
         // exist in this scope's map, then a new one with the NULL value is
diff --git a/build2/test/script/script.cxx b/build2/test/script/script.cxx
index 7abc3b6..17eacaa 100644
--- a/build2/test/script/script.cxx
+++ b/build2/test/script/script.cxx
@@ -8,6 +8,7 @@
 #include <algorithm> // find()
 
 #include <build2/target>
+#include <build2/algorithm>
 
 using namespace std;
 
@@ -390,8 +391,9 @@ namespace build2
       //
       script_base::
       script_base ()
-          : // Enter the test* variables with the same variable types as in
-            // buildfiles.
+          : // Enter the test.* variables with the same variable types as in
+            // buildfiles except for test: while in buildfiles it can be a
+            // target name, in testscripts it should be resolved to a path.
             //
             test_var      (var_pool.insert<path> ("test")),
             options_var   (var_pool.insert<strings> ("test.options")),
@@ -440,24 +442,75 @@ namespace build2
         //
         const_cast<dir_path&> (wd_path) = dir_path (rwd) /= id_path.string ();
 
-        // Unless we have the test variable set on the test or script target,
-        // set it at the script level to the test target's path.
+        // Set the test variable at the script level. We do it even if it's
+        // set in the buildfile since they use different types.
         //
-        // Note that test variable's visibility is target.
-        //
-        if (!find (test_var))
         {
           value& v (assign (test_var));
 
-          // If this is a path-based target, then we use the path. If this is
-          // an alias target (e.g., dir{}), then we use the directory path.
-          // Otherwise, we leave it NULL expecting the testscript to set it to
-          // something appropriate, if used.
+          // Note that the test variable's visibility is target.
           //
-          if (auto* p = tt.is_a<path_target> ())
-            v = p->path ();
-          else if (tt.is_a<alias> ())
-            v = path (tt.dir.string ()); // Strip trailing slash.
+          lookup l (find_in_buildfile ("test", false));
+
+          target* t (nullptr);
+          if (l.defined ())
+          {
+            const name* n (cast_null<name> (l));
+
+            if (n == nullptr)
+              v = nullptr;
+            else if (n->empty ())
+              v = path ();
+            else if (n->simple ())
+            {
+              // Ignore the special 'true' value.
+              //
+              if (n->value != "true")
+                v = path (n->value);
+              else
+                t = &tt;
+            }
+            else if (n->directory ())
+              v = path (n->dir);
+            else
+            {
+              // Must be a target name.
+              //
+              // @@ OUT: what if this is a @-qualified pair or names?
+              //
+              t = &search (*n, tt.base_scope ());
+            }
+          }
+          else
+            // By default we set it to the test target's path.
+            //
+            t = &tt;
+
+          // If this is a path-based target, then we use the path. If this
+          // is an alias target (e.g., dir{}), then we use the directory
+          // path. Otherwise, we leave it NULL expecting the testscript to
+          // set it to something appropriate, if used.
+          //
+          if (t != nullptr)
+          {
+            if (auto* p = t->is_a<path_target> ())
+            {
+              // Do some sanity checks: the target better be up-to-date with
+              // an assigned path.
+              //
+              if (p->path ().empty ())
+                fail << "target " << *p << " specified in the test variable "
+                     << "is out of date" <<
+                  info << "consider specifying it as a prerequisite of " << tt;
+
+              v = p->path ();
+            }
+            else if (t->is_a<alias> ())
+              v = path (t->dir);
+            else if (t != &tt)
+              fail << "target " << *t << " specified in the test variable "
+                   << "is not path-based";
+          }
         }
 
         // Set the special $*, $N variables.
@@ -484,7 +537,7 @@ namespace build2
 
 
       lookup scope::
-      find_in_buildfile (const string& n) const
+      find_in_buildfile (const string& n, bool target_only) const
       {
         // Switch to the corresponding buildfile variable. Note that we don't
         // want to insert a new variable into the pool (we might be running
@@ -506,7 +559,7 @@ namespace build2
           // value. In this case, presumably the override also affects the
           // script target and we will pick it up there. A bit fuzzy.
           //
-          auto p (s.test_target.find_original (var, true));
+          auto p (s.test_target.find_original (var, target_only));
 
           if (p.first)
           {