From 5113d8c29f2e165dabb05c5624055020704ba5fc Mon Sep 17 00:00:00 2001 From: Nicolas Despres Date: Mon, 2 May 2011 16:09:10 +0200 Subject: Add a test for the clean tool. It also fix a bug about the count of removed file reported. --- gen-build-file.py | 2 +- src/clean.cc | 83 ++++++++++------- src/clean.h | 37 +++++++- src/clean_test.cc | 255 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ninja.h | 5 ++ src/ninja_jumble.cc | 14 +++ src/ninja_test.cc | 13 +++ src/test.cc | 11 +++ src/test.h | 2 + 9 files changed, 387 insertions(+), 35 deletions(-) create mode 100644 src/clean_test.cc diff --git a/gen-build-file.py b/gen-build-file.py index 904035c..fb83b20 100755 --- 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 367cd38..5072484 100644 --- 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 4513596..6194aa0 100644 --- a/src/clean.h +++ b/src/clean.h @@ -15,6 +15,8 @@ #ifndef NINJA_CLEAN_H_ #define NINJA_CLEAN_H_ +#include "build.h" + #include #include 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 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 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 180f90c..0f4a029 100644 --- 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 e33fa32..1230546 100644 --- 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 a4ec3a5..e8e7f08 100644 --- 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 mtimes_; vector 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 241c8d3..4dc2f47 100644 --- 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 da1ec11..0af8874 100644 --- 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 files_read_; typedef map FileMap; FileMap files_; + set files_removed_; }; #endif // NINJA_TEST_H_ -- cgit v0.12