From 675d243e0ddeb627c0881b03e7cd5be746a5a7d3 Mon Sep 17 00:00:00 2001
From: Karen Arutyunov <karen@codesynthesis.com>
Date: Tue, 1 Nov 2016 17:25:14 +0300
Subject: Add support for cleanup types to testscript runner

---
 build2/test/script/runner.cxx         | 59 ++++++++++++++-------
 build2/test/script/script             |  5 +-
 tests/test/script/runner/cleanup.test | 97 ++++++++++++++++++++---------------
 3 files changed, 98 insertions(+), 63 deletions(-)

diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx
index 7271234..2af5865 100644
--- a/build2/test/script/runner.cxx
+++ b/build2/test/script/runner.cxx
@@ -215,9 +215,17 @@ namespace build2
 
         for (const auto& c: reverse_iterate (sp.cleanups))
         {
+          cleanup_type t (c.type);
+
+          // Skip whenever the path exists or not.
+          //
+          if (t == cleanup_type::never)
+            continue;
+
           const path& p (c.path);
 
-          // Remove directory if exists and empty. Fail otherwise.
+          // Remove directory if exists and empty. Fail otherwise. Removal of
+          // non-existing directory is not an error for 'maybe' cleanup type.
           //
           if (p.to_directory ())
           {
@@ -232,22 +240,30 @@ namespace build2
             //
             rmdir_status r (rmdir (d, 2));
 
-            if (r != rmdir_status::success)
-              fail (ll) << "registered for cleanup directory " << d
-                        << (r == rmdir_status::not_empty
-                            ? " is not empty"
-                            : " does not exist");
+            if (r == rmdir_status::success ||
+                (r == rmdir_status::not_exist && t == cleanup_type::maybe))
+              continue;
 
-            continue;
+            fail (ll) << "registered for cleanup directory " << d
+                      << (r == rmdir_status::not_empty
+                          ? " is not empty"
+                          : " does not exist");
           }
 
           // Remove directory recursively if not current. Fail otherwise.
-          // Recursive removal of non-existing directory is not an error.
+          // Recursive removal of non-existing directory is not an error for
+          // 'maybe' cleanup type.
           //
           // Note that if some file system entry of non-directory type exists
           // with such a name it is not removed but the operation still
-          // succeeds. The removal of this entry can be handled at the time of
-          // the containing directory removed.
+          // succeeds for 'maybe' cleanup type. The removal of this entry can
+          // be handled at the time of the containing directory cleanup.
+          //
+          // @@ The behavior in the situation described differes for &?a/***
+          //    and &?a/ due to build2::rmdir_r() implementation details which
+          //    checks for directory existence before trying to remove it.
+          //    Shouldn't rmdir_r() behave the same way as rmdir() in regards
+          //    to non-directory removal?
           //
           if (p.leaf ().string () == "***")
           {
@@ -258,21 +274,26 @@ namespace build2
             rmdir_status r (
               rmdir_r (p.directory (), true, static_cast<uint16_t> (2)));
 
-            // Directory is current. That's unlikely to happen but let's keep
-            // for completeness.
-            //
-            if (r == rmdir_status::not_empty)
-              fail (ll) << "registered for cleanup wildcard " << p
-                        << " matches the current directory";
+            if (r == rmdir_status::success ||
+                (r == rmdir_status::not_exist && t == cleanup_type::maybe))
+              continue;
 
-            continue;
+            // The directory is unlikely to be current but let's keep for
+            // completeness.
+            //
+            fail (ll) << "registered for cleanup wildcard " << p
+                      << (r == rmdir_status::not_empty
+                          ? " matches the current directory"
+                          : " doesn't match a directory");
           }
 
-          // Remove file if exists. Fail otherwise.
+          // Remove file if exists. Fail otherwise. Removal of non-existing
+          // file is not an error for 'maybe' cleanup type.
           //
           verify (p, p, "file");
 
-          if (rmfile (p, 2) == rmfile_status::not_exist)
+          if (rmfile (p, 2) == rmfile_status::not_exist &&
+              t == cleanup_type::always)
             fail (ll) << "registered for cleanup file " << p
                       << " does not exist";
         }
diff --git a/build2/test/script/script b/build2/test/script/script
index 9c65b96..5f6b861 100644
--- a/build2/test/script/script
+++ b/build2/test/script/script
@@ -95,7 +95,7 @@ namespace build2
       {
         always, // &foo  - cleanup, fail if does not exist.
         maybe,  // &?foo - cleanup, ignore if does not exist.
-        never   // &!foo - cleanup, ignore if exists.
+        never   // &!foo - don’t cleanup, ignore if doesn’t exist.
       };
 
       // File or directory to be automatically cleaned up at the end of the
@@ -111,8 +111,7 @@ namespace build2
       // dir/**  - remove all files recursively
       // dir/**/ - remove all sub-directories recursively (must be empty)
       // dir/*** - remove directory dir with all files and sub-directories
-      //           recursively (removing non-existent directory is not an
-      //           error)
+      //           recursively
       //
       struct cleanup
       {
diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test
index 200beb9..599bb5c 100644
--- a/tests/test/script/runner/cleanup.test
+++ b/tests/test/script/runner/cleanup.test
@@ -11,56 +11,58 @@ using test
 EOI
 
 b = $build.driver -q --no-column --buildfile - <"./: test{testscript}" \
-  &test/*** test
+  &?test/*** test
 c = cat >>>testscript
 
 # Valid cleanups.
 #
 # @@ TODO: $c <"$* -f a &a" && $b
 #
-: file
+: file-always
 :
 $c <"$* -f a &a";
 $b
 
-: dir1
+: file-maybe
 :
-$c <"$* -d a &a/";
+$c <"$* &?a";
 $b
 
-: dir2
+: file-never
 :
-$c <"$* -d a/b &a/ &a/b/";
+$c <"$* &!a";
 $b
 
-: file-dir
+: dir-always
 :
-$c <"$* -d a/b -f a/b/c &a/ &a/b/ &a/b/c";
+$c <"$* -d a &a/";
 $b
 
-: wildcard1
+: dir-maybe
 :
-$c <"$* -d a/b -f a/b/c &a/***";
+$c <"$* &?a/";
 $b
 
-: wildcard2
+: wildcard-always
 :
-$c <"$* &a/***";
+$c <"$* -d a/b -f a/b/c &a/***";
 $b
 
-: file-dup
+: wildcard-maybe
 :
-$c <"$* -f a &a &a";
+$c <"$* &?a/***";
 $b
 
-: dir-dup
+: order
+: Test that cleanup is performed in registration reversed order
 :
-$c <"$* -d a/b &a/ &a/b/ &a/b/../b/";
+$c <"$* -d a/b &a/ &a/b/";
 $b
 
 # Invalid cleanups.
 #
 : file-not-exists
+: Test cleanup of non-existing file
 :
 $c <"$* &a";
 $b 2>>EOE != 0
@@ -68,13 +70,23 @@ testscript:1: error: registered for cleanup file test/1/a does not exist
 EOE
 
 : file-out-wd
+: Test cleanup of file out of working directory
 :
 $c <"$* &../a";
 $b 2>>EOE != 0
 testscript:1: error: registered for cleanup file test/a is out of working directory test/1/
 EOE
 
+: not-file
+: Test cleanup of directory as a file
+:
+$c <"$* -d a &a";
+$b 2>>EOE != 0
+error: unable to remove file test/1/a: Is a directory
+EOE
+
 : dir-not-exists
+: Test cleanup of non-existing directory
 :
 $c <"$* &a/";
 $b 2>>EOE != 0
@@ -82,57 +94,60 @@ testscript:1: error: registered for cleanup directory test/1/a/ does not exist
 EOE
 
 : dir-out-wd
+: Test cleanup of directory out of working directory
 :
 $c <"$* &../a/";
 $b 2>>EOE != 0
 testscript:1: error: registered for cleanup directory test/a/ is out of working directory test/1/
 EOE
 
-: dir-not-empty1
-:
-$c <"$* -d a/b -f a/b/c";
-$b 2>>EOE != 0
-testscript:1: error: registered for cleanup directory test/1/ is not empty
-EOE
-
-: dir-not-empty2
+: dir-not-empty
+: Test cleanup of non-empty directory
 :
-$c <"$* -d a/b &a/b/";
+$c <"$* -d a -f a/b &a/";
 $b 2>>EOE != 0
-testscript:1: error: registered for cleanup directory test/1/ is not empty
+testscript:1: error: registered for cleanup directory test/1/a/ is not empty
 EOE
 
-: dir-not-empty3
+: not-dir
+: Test cleanup of file as a directory
 :
-$c <"$* -d a/b &a/b/ &a/";
+$c <"$* -f a &a/";
 $b 2>>EOE != 0
-testscript:1: error: registered for cleanup directory test/1/a/ is not empty
+error: unable to remove directory test/1/a/: Not a directory
 EOE
 
-: dir-not-empty4
+: wildcard-not-exists
+: Test cleanup of wildcard not matching any directory
 :
-$c <"$* -f a &a/***";
+$c <"$* &a/***";
 $b 2>>EOE != 0
-testscript:1: error: registered for cleanup directory test/1/ is not empty
+testscript:1: error: registered for cleanup wildcard test/1/a/*** doesn't match a directory
 EOE
 
-: not-file
+: wildcard-out-wd
+: Test cleanup of wildcard out of working directory
 :
-$c <"$* -d a &a";
+$c <"$* &../a/***";
 $b 2>>EOE != 0
-error: unable to remove file test/1/a: Is a directory
+testscript:1: error: registered for cleanup wildcard test/a/*** is out of working directory test/1/
 EOE
 
-: not-dir1
+: implicit-overwrite
+: Test implicit cleanup being overwritten with an explicit one
 :
-$c <"$* -f a &a/";
+$c <"$* -o foo >>>a &!a";
 $b 2>>EOE != 0
-error: unable to remove directory test/1/a/: Not a directory
+testscript:1: error: registered for cleanup directory test/1/ is not empty
 EOE
 
-: wildcard-out-wd
+: explicit-overwrite
+: Test explicit cleanup not being overwritten with an implicit one
 :
-$c <"$* &../a/***";
+$c <<EOO;
+$* &!a;
+$* -o foo >>>a
+EOO
 $b 2>>EOE != 0
-testscript:1: error: registered for cleanup wildcard test/a/*** is out of working directory test/1/
+testscript:2: error: registered for cleanup directory test/1/ is not empty
 EOE
-- 
cgit v1.1