Add support for config.test.output variable

14 files changed, 269 insertions, 123 deletions

diff --git a/build2/test/common b/build2/test/common
index 8e4235b..1a3117a 100644
--- a/build2/test/common
+++ b/build2/test/common
@@ -21,8 +21,8 @@ namespace build2
     {
       // The config.test.output values.
       //
-      output_before before;
-      output_after after;
+      output_before before = output_before::warn;
+      output_after after = output_after::clean;
 
       // The config.test query interface.
       //
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index 33ffb90..6ef2587 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -422,9 +422,16 @@ namespace build2
 
       if (exists (wd))
       {
-        warn << "working directory " << wd << " exists "
-             << (empty (wd) ? "" : "and is not empty ") << "at the beginning "
-             << "of the test";
+        if (before != output_before::clean)
+        {
+          bool fail (before == output_before::fail);
+
+          (fail ? error : warn) << "working directory " << wd << " exists "
+                                << (empty (wd) ? "" : "and is not empty ")
+                                << "at the beginning of the test";
+          if (fail)
+            throw failed ();
+        }
 
         // Remove the directory itself not to confuse the runner which tries
         // to detect when tests stomp on each others feet.
@@ -533,7 +540,7 @@ namespace build2
 
       // Cleanup.
       //
-      if (!one && !mk)
+      if (!one && !mk && after == output_after::clean)
       {
         if (!empty (wd))
           fail << "working directory " << wd << " is not empty at the "
diff --git a/build2/test/script/parser b/build2/test/script/parser
index cb536e6..e018548 100644
--- a/build2/test/script/parser
+++ b/build2/test/script/parser
@@ -184,7 +184,7 @@ namespace build2
         exec_scope_body ();
 
         void
-        exec_lines (lines::iterator, lines::iterator, size_t&, bool);
+        exec_lines (lines::iterator, lines::iterator, size_t&, command_type);
 
         // Customization hooks.
         //
diff --git a/build2/test/script/parser.cxx b/build2/test/script/parser.cxx
index 9d17304..5a33007 100644
--- a/build2/test/script/parser.cxx
+++ b/build2/test/script/parser.cxx
@@ -2866,11 +2866,13 @@ namespace build2
 
         if (test* t = dynamic_cast<test*> (scope_))
         {
-          exec_lines (t->tests_.begin (), t->tests_.end (), li, true);
+          exec_lines (
+            t->tests_.begin (), t->tests_.end (), li, command_type::test);
         }
         else if (group* g = dynamic_cast<group*> (scope_))
         {
-          exec_lines (g->setup_.begin (), g->setup_.end (), li, false);
+          exec_lines (
+            g->setup_.begin (), g->setup_.end (), li, command_type::setup);
 
           atomic_count task_count (0);
           wait_guard wg (task_count);
@@ -3017,7 +3019,8 @@ namespace build2
             }
           }
 
-          exec_lines (g->tdown_.begin (), g->tdown_.end (), li, false);
+          exec_lines (
+            g->tdown_.begin (), g->tdown_.end (), li, command_type::teardown);
         }
         else
           assert (false);
@@ -3028,7 +3031,9 @@ namespace build2
       }
 
       void parser::
-      exec_lines (lines::iterator i, lines::iterator e, size_t& li, bool test)
+      exec_lines (lines::iterator i, lines::iterator e,
+                  size_t& li,
+                  command_type ct)
       {
         token t;
         type tt;
@@ -3096,7 +3101,7 @@ namespace build2
               // We use the 0 index to signal that this is the only command.
               // Note that we only do this for test commands.
               //
-              if (test && li == 0)
+              if (ct == command_type::test && li == 0)
               {
                 lines::iterator j (i);
                 for (++j; j != e && j->type == line_type::var; ++j) ;
@@ -3108,7 +3113,7 @@ namespace build2
                 ++li;
 
               command_expr ce (parse_command_line (t, tt));
-              runner_->run (*scope_, ce, li, ll);
+              runner_->run (*scope_, ce, ct, li, ll);
 
               replay_stop ();
               break;
@@ -3207,7 +3212,7 @@ namespace build2
               if (take)
               {
                 lines::iterator j (next (i, false, false)); // Next if-else.
-                exec_lines (i + 1, j, li, test);
+                exec_lines (i + 1, j, li, ct);
                 i = j->type == line_type::cmd_end ? j : next (j, true, true);
               }
               else
diff --git a/build2/test/script/runner b/build2/test/script/runner
index 56ea834..566b9de 100644
--- a/build2/test/script/runner
+++ b/build2/test/script/runner
@@ -42,7 +42,10 @@ namespace build2
         // testscript. It can be used in diagnostics.
         //
         virtual void
-        run (scope&, const command_expr&, size_t index, const location&) = 0;
+        run (scope&,
+             const command_expr&, command_type,
+             size_t index,
+             const location&) = 0;
 
         virtual bool
         run_if (scope&, const command_expr&, size_t, const location&) = 0;
@@ -66,7 +69,10 @@ namespace build2
         enter (scope&, const location&) override;
 
         virtual void
-        run (scope&, const command_expr&, size_t, const location&) override;
+        run (scope&,
+             const command_expr&, command_type,
+             size_t,
+             const location&) override;
 
         virtual bool
         run_if (scope&, const command_expr&, size_t, const location&) override;
diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 5991546..bcaca44 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -667,136 +667,144 @@ namespace build2
       void default_runner::
       leave (scope& sp, const location& ll)
       {
-        // Remove files and directories in the order opposite to the order of
-        // cleanup registration.
+        // Perform registered cleanups if requested.
         //
-        // Note that we operate with normalized paths here.
-        //
-        for (const auto& c: reverse_iterate (sp.cleanups))
+        if (common_.after == output_after::clean)
         {
-          cleanup_type t (c.type);
-
-          // Skip whenever the path exists or not.
+          // Remove files and directories in the order opposite to the order of
+          // cleanup registration.
           //
-          if (t == cleanup_type::never)
-            continue;
-
-          const path& p (c.path);
-          const path& l (p.leaf ());
-          const string& ls (l.string ());
-
-          // Remove the directory recursively if not current. Fail otherwise.
-          // Recursive removal of non-existing directory is not an error for
-          // 'maybe' cleanup type.
+          // Note that we operate with normalized paths here.
           //
-          if (ls == "***")
+          for (const auto& c: reverse_iterate (sp.cleanups))
           {
-            // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r().
-            //
-            rmdir_status r (
-              rmdir_r (p.directory (), true, static_cast<uint16_t> (2)));
+            cleanup_type t (c.type);
 
-            if (r == rmdir_status::success ||
-                (r == rmdir_status::not_exist && t == cleanup_type::maybe))
+            // Skip whenever the path exists or not.
+            //
+            if (t == cleanup_type::never)
               continue;
 
-            // The directory is unlikely to be current but let's keep for
-            // completeness.
+            const path& p (c.path);
+            const path& l (p.leaf ());
+            const string& ls (l.string ());
+
+            // Remove the directory recursively if not current. Fail otherwise.
+            // Recursive removal of non-existing directory is not an error for
+            // 'maybe' cleanup type.
             //
-            fail (ll) << "registered for cleanup wildcard " << p
-                      << (r == rmdir_status::not_empty
-                          ? " matches the current directory"
-                          : " doesn't match a directory");
-          }
+            if (ls == "***")
+            {
+              // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r().
+              //
+              rmdir_status r (
+                rmdir_r (p.directory (), true, static_cast<uint16_t> (2)));
 
-          // Remove files or directories using wildcard. Removal of sub-entries
-          // of non-existing directory is not an error for 'maybe' cleanup
-          // type.
-          //
-          // Note that only the leaf part of the cleanup wildcard is a pattern
-          // in terms of libbutl::path_search().
-          //
-          if (ls == "*" || ls == "**")
-          {
-            dir_path d (p.directory ());
+              if (r == rmdir_status::success ||
+                  (r == rmdir_status::not_exist && t == cleanup_type::maybe))
+                continue;
 
-            if (t == cleanup_type::always && !dir_exists (d))
+              // The directory is unlikely to be current but let's keep for
+              // completeness.
+              //
               fail (ll) << "registered for cleanup wildcard " << p
-                        << " doesn't match a directory";
+                        << (r == rmdir_status::not_empty
+                            ? " matches the current directory"
+                            : " doesn't match a directory");
+            }
 
-            if (l.to_directory ())
+            // Remove files or directories using wildcard. Removal of
+            // sub-entries of non-existing directory is not an error for
+            // 'maybe' cleanup type.
+            //
+            // Note that only the leaf part of the cleanup wildcard is a
+            // pattern in terms of libbutl::path_search().
+            //
+            if (ls == "*" || ls == "**")
             {
-              auto rm =
-                [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool
+              dir_path d (p.directory ());
+
+              if (t == cleanup_type::always && !dir_exists (d))
+                fail (ll) << "registered for cleanup wildcard " << p
+                          << " doesn't match a directory";
+
+              if (l.to_directory ())
               {
-                if (!interm)
+                auto rm =
+                  [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool
                 {
-                  dir_path sd (path_cast<dir_path> (d / de));
+                  if (!interm)
+                  {
+                    dir_path sd (path_cast<dir_path> (d / de));
 
-                  // We can get not_exist here due to racing conditions, but
-                  // that's ok if somebody did our job.
-                  //
-                  rmdir_status r (rmdir (sd, 2));
+                    // We can get not_exist here due to racing conditions, but
+                    // that's ok if somebody did our job.
+                    //
+                    rmdir_status r (rmdir (sd, 2));
+
+                    if (r != rmdir_status::not_empty)
+                      return true;
 
-                  if (r == rmdir_status::not_empty)
                     fail (ll) << "registered for cleanup directory " << sd
                               << " is not empty" <<
                       info << "wildcard: '" << p << "'";
-                }
+                  }
 
-                return true;
-              };
+                  return true;
+                };
 
-              path_search (l, rm, d);
-            }
-            else
-            {
-              auto rm = [&d] (path&& p, const string&, bool interm) -> bool
+                path_search (l, rm, d);
+              }
+              else
               {
-                if (!interm)
-                  rmfile (d / p, 2); // That's ok if not exists.
+                auto rm = [&d] (path&& p, const string&, bool interm) -> bool
+                {
+                  if (!interm)
+                    rmfile (d / p, 2); // That's ok if not exists.
 
-                return true;
-              };
+                  return true;
+                };
 
-              path_search (l, rm, d);
+                path_search (l, rm, d);
+              }
+
+              continue;
             }
 
-            continue;
-          }
+            // Remove the directory if exists and empty. Fail otherwise.
+            // Removal of non-existing directory is not an error for 'maybe'
+            // cleanup type.
+            //
+            if (p.to_directory ())
+            {
+              dir_path d (path_cast<dir_path> (p));
 
-          // Remove the directory if exists and empty. Fail otherwise. Removal
-          // of non-existing directory is not an error for 'maybe' cleanup
-          // type.
-          //
-          if (p.to_directory ())
-          {
-            dir_path d (path_cast<dir_path> (p));
+              // @@ If 'd' is a file then will fail with a diagnostics having
+              //    no location info. Probably need to add an optional location
+              //    parameter to rmdir() function. The same problem exists for
+              //    a file cleanup when try to rmfile() directory instead of
+              //    file.
+              //
+              rmdir_status r (rmdir (d, 2));
 
-            // @@ If 'd' is a file then will fail with a diagnostics having no
-            //    location info. Probably need to add an optional location
-            //    parameter to rmdir() function. The same problem exists for a
-            //    file cleanup when try to rmfile() directory instead of file.
-            //
-            rmdir_status r (rmdir (d, 2));
+              if (r == rmdir_status::success ||
+                  (r == rmdir_status::not_exist && t == cleanup_type::maybe))
+                continue;
 
-            if (r == rmdir_status::success ||
-                (r == rmdir_status::not_exist && t == cleanup_type::maybe))
-              continue;
+              fail (ll) << "registered for cleanup directory " << d
+                        << (r == rmdir_status::not_empty
+                            ? " is not empty"
+                            : " does not exist");
+            }
 
-            fail (ll) << "registered for cleanup directory " << d
-                      << (r == rmdir_status::not_empty
-                          ? " is not empty"
-                          : " does not exist");
+            // Remove the file if exists. Fail otherwise. Removal of
+            // non-existing file is not an error for 'maybe' cleanup type.
+            //
+            if (rmfile (p, 2) == rmfile_status::not_exist &&
+                t == cleanup_type::always)
+              fail (ll) << "registered for cleanup file " << p
+                        << " does not exist";
           }
-
-          // Remove the file if exists. Fail otherwise. Removal of non-existing
-          // file is not an error for 'maybe' cleanup type.
-          //
-          if (rmfile (p, 2) == rmfile_status::not_exist &&
-              t == cleanup_type::always)
-            fail (ll) << "registered for cleanup file " << p
-                      << " does not exist";
         }
 
         // Return to the parent scope directory or to the out_base one for the
@@ -1609,10 +1617,19 @@ namespace build2
       }
 
       void default_runner::
-      run (scope& sp, const command_expr& expr, size_t li, const location& ll)
+      run (scope& sp,
+           const command_expr& expr, command_type ct,
+           size_t li,
+           const location& ll)
       {
+        // Noop for teardown commands if keeping tests output is requested.
+        //
+        if (ct == command_type::teardown &&
+            common_.after == output_after::keep)
+          return;
+
         if (verb >= 3)
-          text << expr;
+          text << ct << expr;
 
         if (!run_expr (sp, expr, li, ll, true))
           throw failed (); // Assume diagnostics is already printed.
diff --git a/build2/test/script/script b/build2/test/script/script
index 04c54db..c238daf 100644
--- a/build2/test/script/script
+++ b/build2/test/script/script
@@ -313,6 +313,13 @@ namespace build2
       ostream&
       operator<< (ostream&, const command_expr&);
 
+      // command_type
+      //
+      enum class command_type {test, setup, teardown};
+
+      ostream&
+      operator<< (ostream&, command_type);
+
       // description
       //
       struct description
diff --git a/build2/test/script/script.cxx b/build2/test/script/script.cxx
index c5cede8..0fa9dc6 100644
--- a/build2/test/script/script.cxx
+++ b/build2/test/script/script.cxx
@@ -305,6 +305,23 @@ namespace build2
         }
       }
 
+      // command_type
+      //
+      ostream&
+      operator<< (ostream& o, command_type ct)
+      {
+        const char* s (nullptr);
+
+        switch (ct)
+        {
+        case command_type::test:     s = "";  break;
+        case command_type::setup:    s = "+"; break;
+        case command_type::teardown: s = "-"; break;
+        }
+
+        return o << s;
+      }
+
       // redirect
       //
       redirect::
diff --git a/tests/test/script/runner/buildfile b/tests/test/script/runner/buildfile
index c3df228..a990405 100644
--- a/tests/test/script/runner/buildfile
+++ b/tests/test/script/runner/buildfile
@@ -2,7 +2,7 @@
 # copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
 # license   : MIT; see accompanying LICENSE file
 
-./: test{cleanup expr if pipe redirect regex set status} exe{driver} $b
+./: test{cleanup expr if output pipe redirect regex set status} exe{driver} $b
 
 test{*}: target = exe{driver}
 
diff --git a/tests/test/script/runner/output.test b/tests/test/script/runner/output.test
new file mode 100644
index 0000000..f46b6ec
--- /dev/null
+++ b/tests/test/script/runner/output.test
@@ -0,0 +1,87 @@
+# file      : tests/test/script/runner/output.test
+# copyright : Copyright (c) 2014-2017 Code Synthesis Ltd
+# license   : MIT; see accompanying LICENSE file
+
+# Some of the tests below (*/script-wd, before/*) are probably more appropriate
+# for tests/test/script/script-integration/testscript. Nevertheless let's not
+# spread the feature tests among several places.
+#
+
+.include ../common.test
+
+: after
+:
+{
+  : keep
+  :
+  {
+    : cleanup
+    :
+    $c <'touch a' && $b config.test.output=keep;
+    test -f test/1/a
+
+    : teardown
+    :
+    $c <<EOI && $b config.test.output=keep;
+      +touch a &!a
+      -rm a
+      EOI
+    test -f test/a
+
+    : script-wd
+    :
+    : There is no working directory created for a empty script, so add a dummy
+    : assignment here.
+    :
+    $c <'a=b' && $b config.test.output=keep;
+    test -d test
+  }
+
+  : clean
+  :
+  {
+    : cleanup
+    :
+    $c <'touch a' && $b config.test.output=clean;
+    test -f test/1/a == 1
+
+    : teardown
+    :
+    $c <<EOI && $b config.test.output=clean;
+      +touch a &!a
+      -rm a
+      EOI
+    test -f test/a == 1
+
+    : script-wd
+    :
+    : There is no working directory created for a empty script, so add a dummy
+    : assignment here.
+    :
+    $c <'a=b' && $b config.test.output=clean;
+    test -d test == 1
+  }
+}
+
+: before
+:
+{
+  : fail
+  :
+  mkdir test &!test/;
+  $c <'true' && $b config.test.output=fail@clean 2>>/EOE != 0
+    error: working directory test/ exists at the beginning of the test
+    EOE
+
+  : warn
+  :
+  mkdir test &!test/;
+  $c <'true' && $b config.test.output=warn@clean 2>>/EOE
+    warning: working directory test/ exists at the beginning of the test
+    EOE
+
+  : clean
+  :
+  mkdir test &!test/;
+  $c <'true' && $b config.test.output=clean@clean
+}
diff --git a/unit-tests/test/script/parser/command-if.test b/unit-tests/test/script/parser/command-if.test
index ffe2c35..29d1dce 100644
--- a/unit-tests/test/script/parser/command-if.test
+++ b/unit-tests/test/script/parser/command-if.test
@@ -89,7 +89,7 @@
   EOI
   {
     ? true
-    cmd
+    +cmd
   }
   EOO
 
@@ -102,7 +102,7 @@
   EOI
   {
     ? true
-    cmd
+    -cmd
   }
   EOO
 }
diff --git a/unit-tests/test/script/parser/driver.cxx b/unit-tests/test/script/parser/driver.cxx
index 5872fb8..772bc10 100644
--- a/unit-tests/test/script/parser/driver.cxx
+++ b/unit-tests/test/script/parser/driver.cxx
@@ -90,11 +90,11 @@ namespace build2
 
         virtual void
         run (scope&,
-             const command_expr& e,
+             const command_expr& e, command_type t,
              size_t i,
              const location&) override
         {
-          cout << ind_ << e;
+          cout << ind_ << t << e;
 
           if (line_)
             cout << " # " << i;
diff --git a/unit-tests/test/script/parser/scope-if.test b/unit-tests/test/script/parser/scope-if.test
index 4f524cf..2867107 100644
--- a/unit-tests/test/script/parser/scope-if.test
+++ b/unit-tests/test/script/parser/scope-if.test
@@ -489,7 +489,7 @@ else
 -tdown          # 8
 EOI
 {
-  setup # 1
+  +setup # 1
   ? false one # 2
   ? false two # 3
   ? true # 4
@@ -501,7 +501,7 @@ EOI
   {
     cmd2 # 0
   }
-  tdown # 8
+  -tdown # 8
 }
 EOO
 
diff --git a/unit-tests/test/script/parser/scope.test b/unit-tests/test/script/parser/scope.test
index 144dc54..437a3dc 100644
--- a/unit-tests/test/script/parser/scope.test
+++ b/unit-tests/test/script/parser/scope.test
@@ -115,7 +115,7 @@ $* foo.test <'cmd "$~"' >~"%cmd '?.+[/\\\\]test-driver[/\\\\]foo[/\\\\]1'?%"
     EOI
     {
       { # 1
-        setup
+        +setup
         { # 1/4
           cmd abc
         }