Add a test for the clean tool.

It also fix a bug about the count of removed file reported.

diff --git a/gen-build-file.py b/gen-build-file.py
index 904035ce5be446d79b5ae4f7f26276e0f56eb1f1..fb83b202d37aac5fac64a934f5d1a98fc757563a 100755 (executable)
--- a/gen-build-file.py
+++ b/gen-build-file.py
@@ -94,7 +94,7 @@ n.build('ninja', 'link', objs + ninja_lib)
 n.comment('Tests all build into ninja_test executable.')
 objs = []
 for name in ['build_test', 'build_log_test', 'graph_test', 'ninja_test',
-             'parsers_test', 'subprocess_test', 'util_test',
+             'parsers_test', 'subprocess_test', 'util_test', 'clean_test',
              'test']:
     objs += cxx(name)
 ldflags = '-lgtest -lgtest_main -lpthread'

diff --git a/src/clean.cc b/src/clean.cc
index 367cd38b7ac302ea11d49df3c0d6e799ab6f1ac5..5072484780c622d69944ae418270527db3fa6329 100644 (file)
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -27,50 +27,42 @@
 
 Cleaner::Cleaner(State* state, const BuildConfig& config)
   : state_(state)
-  , verbose_(config.verbosity == BuildConfig::VERBOSE || config.dry_run)
-  , dry_run_(config.dry_run)
+  , config_(config)
   , removed_()
+  , cleaned_files_count_(0)
+  , disk_interface_(new RealDiskInterface)
+{
+}
+
+Cleaner::Cleaner(State* state,
+                 const BuildConfig& config,
+                 DiskInterface* disk_interface)
+  : state_(state)
+  , config_(config)
+  , removed_()
+  , cleaned_files_count_(0)
+  , disk_interface_(disk_interface)
 {
 }
 
 bool Cleaner::RemoveFile(const string& path) {
-  if (remove(path.c_str()) < 0) {
-    switch (errno) {
-      case ENOENT:
-        return false;
-      default:
-        Error("remove(%s): %s", path.c_str(), strerror(errno));
-        return false;
-    }
-  } else {
-    return true;
-  }
+  return disk_interface_->RemoveFile(path);
 }
 
 bool Cleaner::FileExists(const string& path) {
-  struct stat st;
-  if (stat(path.c_str(), &st) < 0) {
-    switch (errno) {
-      case ENOENT:
-        return false;
-      default:
-        Error("stat(%s): %s", path.c_str(), strerror(errno));
-        return false;
-    }
-  } else {
-    return true;
-  }
+  return disk_interface_->Stat(path) > 0;
 }
 
 void Cleaner::Report(const string& path) {
-  if (verbose_)
+  ++cleaned_files_count_;
+  if (IsVerbose())
     printf("Remove %s\n", path.c_str());
 }
 
 void Cleaner::Remove(const string& path) {
   if (!IsAlreadyRemoved(path)) {
     removed_.insert(path);
-    if (dry_run_) {
+    if (config_.dry_run) {
       if (FileExists(path))
         Report(path);
     } else {
@@ -86,15 +78,19 @@ bool Cleaner::IsAlreadyRemoved(const string& path) {
 }
 
 void Cleaner::PrintHeader() {
+  if (config_.verbosity == BuildConfig::QUIET)
+    return;
   printf("Cleaning...");
-  if (verbose_)
+  if (IsVerbose())
     printf("\n");
   else
     printf(" ");
 }
 
 void Cleaner::PrintFooter() {
-  printf("%d files.\n", (int)removed_.size());
+  if (config_.verbosity == BuildConfig::QUIET)
+    return;
+  printf("%d files.\n", cleaned_files_count_);
 }
 
 void Cleaner::CleanAll() {
@@ -128,6 +124,18 @@ void Cleaner::CleanTarget(Node* target) {
   PrintFooter();
 }
 
+int Cleaner::CleanTarget(const char* target) {
+  assert(target);
+  Node* node = state_->LookupNode(target);
+  if (node) {
+    CleanTarget(node);
+    return 0;
+  } else {
+    Error("unknown target '%s'", target);
+    return 1;
+  }
+}
+
 int Cleaner::CleanTargets(int target_count, char* targets[]) {
   int status = 0;
   PrintHeader();
@@ -135,7 +143,7 @@ int Cleaner::CleanTargets(int target_count, char* targets[]) {
     const char* target_name = targets[i];
     Node* target = state_->LookupNode(target_name);
     if (target) {
-      if (verbose_)
+      if (IsVerbose())
         printf("Target %s\n", target_name);
       DoCleanTarget(target);
     } else {
@@ -168,6 +176,19 @@ void Cleaner::CleanRule(const Rule* rule) {
   PrintFooter();
 }
 
+int Cleaner::CleanRule(const char* rule) {
+  assert(rule);
+
+  const Rule* r = state_->LookupRule(rule);
+  if (r) {
+    CleanRule(r);
+    return 0;
+  } else {
+    Error("unknown rule '%s'", rule);
+    return 1;
+  }
+}
+
 int Cleaner::CleanRules(int rule_count, char* rules[]) {
   assert(rules);
 
@@ -177,7 +198,7 @@ int Cleaner::CleanRules(int rule_count, char* rules[]) {
     const char* rule_name = rules[i];
     const Rule* rule = state_->LookupRule(rule_name);
     if (rule) {
-      if (verbose_)
+      if (IsVerbose())
         printf("Rule %s\n", rule_name);
       DoCleanRule(rule);
     } else {

diff --git a/src/clean.h b/src/clean.h
index 4513596f0b641a6847eebcf97598f9be49a19a4e..6194aa04e225e575a430c891a505599572b2eb65 100644 (file)
--- a/src/clean.h
+++ b/src/clean.h
@@ -15,6 +15,8 @@
 #ifndef NINJA_CLEAN_H_
 #define NINJA_CLEAN_H_
 
+#include "build.h"
+
 #include <string>
 #include <set>
 using namespace std;
@@ -23,15 +25,27 @@ struct State;
 struct BuildConfig;
 struct Node;
 struct Rule;
+struct DiskInterface;
 
 class Cleaner
 {
 public:
-  /// Constructor.
+  /// Build a cleaner object with a real disk interface.
   Cleaner(State* state, const BuildConfig& config);
 
+  /// Build a cleaner object with the given @a disk_interface
+  /// (Useful for testing).
+  Cleaner(State* state,
+          const BuildConfig& config,
+          DiskInterface* disk_interface);
+
   /// Clean the given @a target and all the file built for it.
   void CleanTarget(Node* target);
+  /// Clean the given target @a target.
+  /// @return non-zero if an error occurs.
+  int CleanTarget(const char* target);
+  /// Clean the given target @a targets.
+  /// @return non-zero if an error occurs.
   int CleanTargets(int target_count, char* targets[]);
 
   /// Clean all built files.
@@ -39,8 +53,24 @@ public:
 
   /// Clean all the file built with the given rule @a rule.
   void CleanRule(const Rule* rule);
+  /// Clean the file produced by the given @a rule.
+  /// @return non-zero if an error occurs.
+  int CleanRule(const char* rule);
+  /// Clean the file produced by the given @a rules.
+  /// @return non-zero if an error occurs.
   int CleanRules(int rule_count, char* rules[]);
 
+  /// @return the number of file cleaned.
+  int cleaned_files_count() const {
+    return cleaned_files_count_;
+  }
+
+  /// @return whether the cleaner is in verbose mode.
+  bool IsVerbose() const {
+    return (config_.verbosity != BuildConfig::QUIET
+            && (config_.verbosity == BuildConfig::VERBOSE || config_.dry_run));
+  }
+
 private:
   /// Remove the file @a path.
   /// @return whether the file has been removed.
@@ -60,9 +90,10 @@ private:
 
 private:
   State* state_;
-  bool verbose_;
-  bool dry_run_;
+  BuildConfig config_;
   set<string> removed_;
+  int cleaned_files_count_;
+  DiskInterface* disk_interface_;
 };
 
 #endif  // NINJA_CLEAN_H_

diff --git a/src/clean_test.cc b/src/clean_test.cc
new file mode 100644 (file)
index 0000000..0e2114e
--- /dev/null
+++ b/src/clean_test.cc
@@ -0,0 +1,255 @@
+// Copyright 2011 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "clean.h"
+#include "build.h"
+
+#include "test.h"
+
+struct CleanTest : public StateTestWithBuiltinRules {
+  VirtualFileSystem fs_;
+  BuildConfig config_;
+  virtual void SetUp() {
+    config_.verbosity = BuildConfig::QUIET;
+  }
+};
+
+TEST_F(CleanTest, CleanAll) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build in1: cat src1\n"
+"build out1: cat in1\n"
+"build in2: cat src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    cleaner.CleanAll();
+    EXPECT_EQ(4, cleaner.cleaned_files_count());
+    EXPECT_EQ(4, fs_.files_removed_.size());
+  }
+
+  // Check they are removed.
+  EXPECT_EQ(0, fs_.Stat("in1"));
+  EXPECT_EQ(0, fs_.Stat("out1"));
+  EXPECT_EQ(0, fs_.Stat("in2"));
+  EXPECT_EQ(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    cleaner.CleanAll();
+    EXPECT_EQ(0, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}
+
+TEST_F(CleanTest, CleanAllDryRun) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build in1: cat src1\n"
+"build out1: cat in1\n"
+"build in2: cat src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  config_.dry_run = true;
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    cleaner.CleanAll();
+    EXPECT_EQ(4, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+
+  // Check they are not removed.
+  EXPECT_NE(0, fs_.Stat("in1"));
+  EXPECT_NE(0, fs_.Stat("out1"));
+  EXPECT_NE(0, fs_.Stat("in2"));
+  EXPECT_NE(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    cleaner.CleanAll();
+    EXPECT_EQ(4, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}
+
+TEST_F(CleanTest, CleanTarget) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build in1: cat src1\n"
+"build out1: cat in1\n"
+"build in2: cat src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanTarget("out1"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(2, fs_.files_removed_.size());
+  }
+
+  // Check they are removed.
+  EXPECT_EQ(0, fs_.Stat("in1"));
+  EXPECT_EQ(0, fs_.Stat("out1"));
+  EXPECT_NE(0, fs_.Stat("in2"));
+  EXPECT_NE(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanTarget("out1"));
+    EXPECT_EQ(0, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}
+
+TEST_F(CleanTest, CleanTargetDryRun) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"build in1: cat src1\n"
+"build out1: cat in1\n"
+"build in2: cat src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  config_.dry_run = true;
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanTarget("out1"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+
+  // Check they are removed.
+  EXPECT_NE(0, fs_.Stat("in1"));
+  EXPECT_NE(0, fs_.Stat("out1"));
+  EXPECT_NE(0, fs_.Stat("in2"));
+  EXPECT_NE(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanTarget("out1"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}
+
+TEST_F(CleanTest, CleanRule) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cat_e\n"
+"  command = cat -e $in > $out\n"
+"build in1: cat_e src1\n"
+"build out1: cat in1\n"
+"build in2: cat_e src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(2, fs_.files_removed_.size());
+  }
+
+  // Check they are removed.
+  EXPECT_EQ(0, fs_.Stat("in1"));
+  EXPECT_NE(0, fs_.Stat("out1"));
+  EXPECT_EQ(0, fs_.Stat("in2"));
+  EXPECT_NE(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
+    EXPECT_EQ(0, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}
+
+TEST_F(CleanTest, CleanRuleDryRun) {
+  ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+"rule cat_e\n"
+"  command = cat -e $in > $out\n"
+"build in1: cat_e src1\n"
+"build out1: cat in1\n"
+"build in2: cat_e src2\n"
+"build out2: cat in2\n"));
+  fs_.Create("in1", 1, "");
+  fs_.Create("out1", 1, "");
+  fs_.Create("in2", 1, "");
+  fs_.Create("out2", 1, "");
+
+  config_.dry_run = true;
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+
+  // Check they are removed.
+  EXPECT_NE(0, fs_.Stat("in1"));
+  EXPECT_NE(0, fs_.Stat("out1"));
+  EXPECT_NE(0, fs_.Stat("in2"));
+  EXPECT_NE(0, fs_.Stat("out2"));
+  fs_.files_removed_.clear();
+
+  {
+    Cleaner cleaner(&state_, config_, &fs_);
+
+    ASSERT_EQ(0, cleaner.cleaned_files_count());
+    ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
+    EXPECT_EQ(2, cleaner.cleaned_files_count());
+    EXPECT_EQ(0, fs_.files_removed_.size());
+  }
+}

diff --git a/src/ninja.h b/src/ninja.h
index 180f90c36f03bc6b1cc3592a1d847b74ccd556f6..0f4a0290a6f07ef92225ac60e28a06cb75c34f15 100644 (file)
--- a/src/ninja.h
+++ b/src/ninja.h
@@ -49,6 +49,10 @@ struct DiskInterface {
   virtual bool MakeDir(const string& path) = 0;
   /// Read a file to a string.  Fill in |err| on error.
   virtual string ReadFile(const string& path, string* err) = 0;
+  /// Remove the file named @a path. It behaves like 'rm -f path' so no errors
+  /// are reported if it does not exists.
+  /// @returns whether the file has been removed.
+  virtual bool RemoveFile(const string& path) = 0;
 
   /// Create all the parent directories for path; like mkdir -p
   /// `basename path`.
@@ -61,6 +65,7 @@ struct RealDiskInterface : public DiskInterface {
   virtual int Stat(const string& path);
   virtual bool MakeDir(const string& path);
   virtual string ReadFile(const string& path, string* err);
+  virtual bool RemoveFile(const string& path);
 };
 
 /// Mapping of path -> FileStat.

diff --git a/src/ninja_jumble.cc b/src/ninja_jumble.cc
index e33fa32c674077b5d1d90893d51ab8bd7fb20188..1230546fb277a510613ea1a4c895ae72ec005854 100644 (file)
--- a/src/ninja_jumble.cc
+++ b/src/ninja_jumble.cc
@@ -107,6 +107,20 @@ bool RealDiskInterface::MakeDir(const string& path) {
   return true;
 }
 
+bool RealDiskInterface::RemoveFile(const string& path) {
+  if (remove(path.c_str()) < 0) {
+    switch (errno) {
+      case ENOENT:
+        return false;
+      default:
+        Error("remove(%s): %s", path.c_str(), strerror(errno));
+        return false;
+    }
+  } else {
+    return true;
+  }
+}
+
 FileStat* StatCache::GetFile(const string& path) {
   Paths::iterator i = paths_.find(path);
   if (i != paths_.end())

diff --git a/src/ninja_test.cc b/src/ninja_test.cc
index a4ec3a50fb2b42fa1d058bde61cc93e1bca47e4e..e8e7f086e000c7b3cc25992be8a8f74a699cf330 100644 (file)
--- a/src/ninja_test.cc
+++ b/src/ninja_test.cc
@@ -114,6 +114,10 @@ struct StatTest : public StateTestWithBuiltinRules,
     assert(false);
     return "";
   }
+  virtual bool RemoveFile(const string& path) {
+    assert(false);
+    return false;
+  }
 
   map<string, time_t> mtimes_;
   vector<string> stats_;
@@ -252,3 +256,12 @@ TEST_F(DiskInterfaceTest, ReadFile) {
 TEST_F(DiskInterfaceTest, MakeDirs) {
   EXPECT_TRUE(disk_.MakeDirs("path/with/double//slash/"));
 }
+
+TEST_F(DiskInterfaceTest, RemoveFile) {
+  const char* kFileName = "file-to-remove";
+  string cmd = "touch ";
+  cmd += kFileName;
+  ASSERT_EQ(0, system(cmd.c_str()));
+  EXPECT_TRUE(disk_.RemoveFile(kFileName));
+  EXPECT_FALSE(disk_.RemoveFile(kFileName));
+}

diff --git a/src/test.cc b/src/test.cc
index 241c8d343ba8c8904528a6bc8918bd3b0e6cd5f4..4dc2f4713e2423a43c2d5b01dff0da5f60465053 100644 (file)
--- a/src/test.cc
+++ b/src/test.cc
@@ -58,3 +58,14 @@ string VirtualFileSystem::ReadFile(const string& path, string* err) {
     return i->second.contents;
   return "";
 }
+
+bool VirtualFileSystem::RemoveFile(const string& path) {
+  FileMap::iterator i = files_.find(path);
+  if (i != files_.end()) {
+    files_.erase(i);
+    files_removed_.insert(path);
+    return true;
+  } else {
+    return false;
+  }
+}

diff --git a/src/test.h b/src/test.h
index da1ec115b7a4f793ee6d22da82589275b39676b2..0af8874f3e37be60bcf05e6ea4c9e27395bdc22b 100644 (file)
--- a/src/test.h
+++ b/src/test.h
@@ -45,6 +45,7 @@ struct VirtualFileSystem : public DiskInterface {
   virtual int Stat(const string& path);
   virtual bool MakeDir(const string& path);
   virtual string ReadFile(const string& path, string* err);
+  virtual bool RemoveFile(const string& path);
 
   /// An entry for a single in-memory file.
   struct Entry {
@@ -56,6 +57,7 @@ struct VirtualFileSystem : public DiskInterface {
   vector<string> files_read_;
   typedef map<string, Entry> FileMap;
   FileMap files_;
+  set<string> files_removed_;
 };
 
 #endif // NINJA_TEST_H_