Permalink
Browse files

Implement cleanup-on-interrupt

This causes us to clean up by deleting any output files belonging
to currently-running commands before we quit if we are interrupted
(either by Ctrl-C or by a command failing).

Fixes issue #110.
  • Loading branch information...
1 parent b07e183 commit 85ff781fa30fff63c01ccd30faaad39d766e1505 @pcc pcc committed Nov 13, 2011
Showing with 318 additions and 84 deletions.
  1. +59 −17 src/build.cc
  2. +13 −2 src/build.h
  3. +10 −5 src/build_test.cc
  4. +24 −0 src/exit_status.h
  5. +55 −8 src/subprocess-win32.cc
  6. +93 −27 src/subprocess.cc
  7. +24 −9 src/subprocess.h
  8. +40 −16 src/subprocess_test.cc
View
76 src/build.cc
@@ -396,35 +396,52 @@ struct RealCommandRunner : public CommandRunner {
virtual ~RealCommandRunner() {}
virtual bool CanRunMore();
virtual bool StartCommand(Edge* edge);
- virtual Edge* WaitForCommand(bool* success, string* output);
+ virtual Edge* WaitForCommand(ExitStatus* status, string* output);
+ virtual vector<Edge*> GetActiveEdges();
+ virtual void Abort();
const BuildConfig& config_;
SubprocessSet subprocs_;
map<Subprocess*, Edge*> subproc_to_edge_;
};
+vector<Edge*> RealCommandRunner::GetActiveEdges() {
+ vector<Edge*> edges;
+ for (map<Subprocess*, Edge*>::iterator i = subproc_to_edge_.begin();
+ i != subproc_to_edge_.end(); ++i)
+ edges.push_back(i->second);
+ return edges;
+}
+
+void RealCommandRunner::Abort() {
+ subprocs_.Clear();
+}
+
bool RealCommandRunner::CanRunMore() {
return ((int)subprocs_.running_.size()) < config_.parallelism;
}
bool RealCommandRunner::StartCommand(Edge* edge) {
string command = edge->EvaluateCommand();
- Subprocess* subproc = new Subprocess;
- subproc_to_edge_.insert(make_pair(subproc, edge));
- if (!subproc->Start(&subprocs_, command))
+ Subprocess* subproc = subprocs_.Add(command);
+ if (!subproc)
return false;
-
- subprocs_.Add(subproc);
+ subproc_to_edge_.insert(make_pair(subproc, edge));
+
return true;
}
-Edge* RealCommandRunner::WaitForCommand(bool* success, string* output) {
+Edge* RealCommandRunner::WaitForCommand(ExitStatus* status, string* output) {
Subprocess* subproc;
while ((subproc = subprocs_.NextFinished()) == NULL) {
- subprocs_.DoWork();
+ bool interrupted = subprocs_.DoWork();
+ if (interrupted) {
+ *status = ExitInterrupted;
+ return 0;
+ }
}
- *success = subproc->Finish();
+ *status = subproc->Finish();
*output = subproc->GetOutput();
map<Subprocess*, Edge*>::iterator i = subproc_to_edge_.find(subproc);
@@ -445,10 +462,12 @@ struct DryRunCommandRunner : public CommandRunner {
finished_.push(edge);
return true;
}
- virtual Edge* WaitForCommand(bool* success, string* output) {
- if (finished_.empty())
+ virtual Edge* WaitForCommand(ExitStatus* status, string* /* output */) {
+ if (finished_.empty()) {
+ *status = ExitFailure;
return NULL;
- *success = true;
+ }
+ *status = ExitSuccess;
Edge* edge = finished_.front();
finished_.pop();
return edge;
@@ -461,13 +480,29 @@ Builder::Builder(State* state, const BuildConfig& config)
: state_(state), config_(config) {
disk_interface_ = new RealDiskInterface;
if (config.dry_run)
- command_runner_ = new DryRunCommandRunner;
+ command_runner_.reset(new DryRunCommandRunner);
else
- command_runner_ = new RealCommandRunner(config);
+ command_runner_.reset(new RealCommandRunner(config));
status_ = new BuildStatus(config);
log_ = state->build_log_;
}
+Builder::~Builder() {
+ if (command_runner_.get()) {
+ vector<Edge*> active_edges = command_runner_->GetActiveEdges();
+ command_runner_->Abort();
+
+ for (vector<Edge*>::iterator i = active_edges.begin();
+ i != active_edges.end(); ++i) {
+ for (vector<Node*>::iterator ni = (*i)->outputs_.begin();
+ ni != (*i)->outputs_.end(); ++ni)
+ disk_interface_->RemoveFile((*ni)->path());
+ if (!(*i)->rule_->depfile_.empty())
+ disk_interface_->RemoveFile((*i)->EvaluateDepFile());
+ }
+ }
+}
+
Node* Builder::AddTarget(const string& name, string* err) {
Node* node = state_->LookupNode(name);
if (!node) {
@@ -533,10 +568,11 @@ bool Builder::Build(string* err) {
// See if we can reap any finished commands.
if (pending_commands) {
- bool success;
+ ExitStatus status;
string output;
- Edge* edge;
- if ((edge = command_runner_->WaitForCommand(&success, &output))) {
+ Edge* edge = command_runner_->WaitForCommand(&status, &output);
+ if (edge && status != ExitInterrupted) {
+ bool success = (status == ExitSuccess);
--pending_commands;
FinishEdge(edge, success, output);
if (!success) {
@@ -552,6 +588,12 @@ bool Builder::Build(string* err) {
// We made some progress; start the main loop over.
continue;
}
+
+ if (status == ExitInterrupted) {
+ status_->BuildFinished();
+ *err = "interrupted by user";
+ return false;
+ }
}
// If we get here, we can neither enqueue new commands nor are any running.
View
15 src/build.h
@@ -20,8 +20,11 @@
#include <string>
#include <queue>
#include <vector>
+#include <memory>
using namespace std;
+#include "exit_status.h"
+
struct BuildLog;
struct Edge;
struct DiskInterface;
@@ -87,7 +90,9 @@ struct CommandRunner {
virtual bool CanRunMore() = 0;
virtual bool StartCommand(Edge* edge) = 0;
/// Wait for a command to complete.
- virtual Edge* WaitForCommand(bool* success, string* output) = 0;
+ virtual Edge* WaitForCommand(ExitStatus* status, string* output) = 0;
+ virtual vector<Edge*> GetActiveEdges() { return vector<Edge*>(); }
+ virtual void Abort() {}
};
/// Options (e.g. verbosity, parallelism) passed to a build.
@@ -109,6 +114,7 @@ struct BuildConfig {
/// Builder wraps the build process: starting commands, updating status.
struct Builder {
Builder(State* state, const BuildConfig& config);
+ ~Builder();
Node* AddTarget(const string& name, string* err);
@@ -130,9 +136,14 @@ struct Builder {
const BuildConfig& config_;
Plan plan_;
DiskInterface* disk_interface_;
- CommandRunner* command_runner_;
+ auto_ptr<CommandRunner> command_runner_;
struct BuildStatus* status_;
struct BuildLog* log_;
+
+private:
+ // Unimplemented copy ctor and operator= ensure we don't copy the auto_ptr.
+ Builder(const Builder &other); // DO NOT IMPLEMENT
+ void operator=(const Builder &other); // DO NOT IMPLEMENT
};
#endif // NINJA_BUILD_H_
View
15 src/build_test.cc
@@ -181,7 +181,7 @@ struct BuildTest : public StateTestWithBuiltinRules,
BuildTest() : config_(MakeConfig()), builder_(&state_, config_), now_(1),
last_command_(NULL) {
builder_.disk_interface_ = &fs_;
- builder_.command_runner_ = this;
+ builder_.command_runner_.reset(this);
AssertParse(&state_,
"build cat1: cat in1\n"
"build cat2: cat in1 in2\n"
@@ -191,13 +191,17 @@ struct BuildTest : public StateTestWithBuiltinRules,
fs_.Create("in2", now_, "");
}
+ ~BuildTest() {
+ builder_.command_runner_.release();
+ }
+
// Mark a path dirty.
void Dirty(const string& path);
// CommandRunner impl
virtual bool CanRunMore();
virtual bool StartCommand(Edge* edge);
- virtual Edge* WaitForCommand(bool* success, string* output);
+ virtual Edge* WaitForCommand(ExitStatus* status, string* output);
BuildConfig MakeConfig() {
BuildConfig config;
@@ -251,15 +255,16 @@ bool BuildTest::StartCommand(Edge* edge) {
return true;
}
-Edge* BuildTest::WaitForCommand(bool* success, string* output) {
+Edge* BuildTest::WaitForCommand(ExitStatus* status, string* /* output */) {
if (Edge* edge = last_command_) {
if (edge->rule().name() == "fail")
- *success = false;
+ *status = ExitFailure;
else
- *success = true;
+ *status = ExitSuccess;
last_command_ = NULL;
return edge;
}
+ *status = ExitFailure;
return NULL;
}
View
24 src/exit_status.h
@@ -0,0 +1,24 @@
+// 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.
+
+#ifndef NINJA_EXIT_STATUS_H_
+#define NINJA_EXIT_STATUS_H_
+
+enum ExitStatus {
+ ExitSuccess,
+ ExitFailure,
+ ExitInterrupted
+};
+
+#endif // NINJA_EXIT_STATUS_H_
View
63 src/subprocess-win32.cc
@@ -32,6 +32,10 @@ Subprocess::Subprocess() : child_(NULL) , overlapped_() {
}
Subprocess::~Subprocess() {
+ if (pipe_) {
+ if (!CloseHandle(pipe_))
+ Win32Fatal("CloseHandle");
+ }
// Reap child if forgotten.
if (child_)
Finish();
@@ -92,7 +96,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
// Do not prepend 'cmd /c' on Windows, this breaks command
// lines greater than 8,191 chars.
if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL,
- /* inherit handles */ TRUE, 0,
+ /* inherit handles */ TRUE, CREATE_NEW_PROCESS_GROUP,
NULL, NULL,
&startup_info, &process_info)) {
DWORD error = GetLastError();
@@ -149,10 +153,9 @@ void Subprocess::OnPipeReady() {
// function again later and get them at that point.
}
-bool Subprocess::Finish() {
- if (! child_) {
- return false;
- }
+ExitStatus Subprocess::Finish() {
+ if (!child_)
+ return ExitFailure;
// TODO: add error handling for all of these.
WaitForSingleObject(child_, INFINITE);
@@ -163,7 +166,9 @@ bool Subprocess::Finish() {
CloseHandle(child_);
child_ = NULL;
- return exit_code == 0;
+ return exit_code == 0 ? ExitSuccess :
+ exit_code == CONTROL_C_EXIT ? ExitInterrupted :
+ ExitFailure;
}
bool Subprocess::Done() const {
@@ -174,24 +179,47 @@ const string& Subprocess::GetOutput() const {
return buf_;
}
+HANDLE SubprocessSet::ioport_;
+
SubprocessSet::SubprocessSet() {
ioport_ = ::CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 1);
if (!ioport_)
Win32Fatal("CreateIoCompletionPort");
+ if (!SetConsoleCtrlHandler(NotifyInterrupted, TRUE))
+ Win32Fatal("SetConsoleCtrlHandler");
}
SubprocessSet::~SubprocessSet() {
+ Clear();
+
+ SetConsoleCtrlHandler(NotifyInterrupted, FALSE);
CloseHandle(ioport_);
}
-void SubprocessSet::Add(Subprocess* subprocess) {
+BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) {
+ if (dwCtrlType == CTRL_C_EVENT || dwCtrlType == CTRL_BREAK_EVENT) {
+ if (!PostQueuedCompletionStatus(ioport_, 0, 0, NULL))
+ Win32Fatal("PostQueuedCompletionStatus");
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+Subprocess *SubprocessSet::Add(const string &command) {
+ Subprocess *subprocess = new Subprocess;
+ if (!subprocess->Start(this, command)) {
+ delete subprocess;
+ return 0;
+ }
if (subprocess->child_)
running_.push_back(subprocess);
else
finished_.push(subprocess);
+ return subprocess;
}
-void SubprocessSet::DoWork() {
+bool SubprocessSet::DoWork() {
DWORD bytes_read;
Subprocess* subproc;
OVERLAPPED* overlapped;
@@ -202,6 +230,10 @@ void SubprocessSet::DoWork() {
Win32Fatal("GetQueuedCompletionStatus");
}
+ if (!subproc) // A NULL subproc indicates that we were interrupted and is
+ // delivered by NotifyInterrupted above.
+ return true;
+
subproc->OnPipeReady();
if (subproc->Done()) {
@@ -212,6 +244,8 @@ void SubprocessSet::DoWork() {
running_.resize(end - running_.begin());
}
}
+
+ return false;
}
Subprocess* SubprocessSet::NextFinished() {
@@ -221,3 +255,16 @@ Subprocess* SubprocessSet::NextFinished() {
finished_.pop();
return subproc;
}
+
+void SubprocessSet::Clear() {
+ for (vector<Subprocess*>::iterator i = running_.begin();
+ i != running_.end(); ++i) {
+ if ((*i)->child_)
+ if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId((*i)->child_)))
+ Win32Fatal("GenerateConsoleCtrlEvent");
+ }
+ for (vector<Subprocess*>::iterator i = running_.begin();
+ i != running_.end(); ++i)
+ delete *i;
+ running_.clear();
+}
View
120 src/subprocess.cc
@@ -42,6 +42,10 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
if (pipe(output_pipe) < 0)
Fatal("pipe: %s", strerror(errno));
fd_ = output_pipe[0];
+ // fd_ may be a member of the pselect set in SubprocessSet::DoWork. Check
+ // that it falls below the system limit.
+ if (fd_ >= FD_SETSIZE)
+ Fatal("pipe: %s", strerror(EMFILE));
SetCloseOnExec(fd_);
pid_ = fork();
@@ -54,6 +58,14 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
// Track which fd we use to report errors on.
int error_pipe = output_pipe[1];
do {
+ if (setpgid(0, 0) < 0)
+ break;
+
+ if (sigaction(SIGINT, &set->old_act_, 0) < 0)
+ break;
+ if (sigprocmask(SIG_SETMASK, &set->old_mask_, 0) < 0)
+ break;
+
// Open /dev/null over stdin.
int devnull = open("/dev/null", O_WRONLY);
if (devnull < 0)
@@ -100,7 +112,7 @@ void Subprocess::OnPipeReady() {
}
}
-bool Subprocess::Finish() {
+ExitStatus Subprocess::Finish() {
assert(pid_ != -1);
int status;
if (waitpid(pid_, &status, 0) < 0)
@@ -110,9 +122,12 @@ bool Subprocess::Finish() {
if (WIFEXITED(status)) {
int exit = WEXITSTATUS(status);
if (exit == 0)
- return true;
+ return ExitSuccess;
+ } else if (WIFSIGNALED(status)) {
+ if (WTERMSIG(status) == SIGINT)
+ return ExitInterrupted;
}
- return false;
+ return ExitFailure;
}
bool Subprocess::Done() const {
@@ -123,48 +138,89 @@ const string& Subprocess::GetOutput() const {
return buf_;
}
-SubprocessSet::SubprocessSet() {}
-SubprocessSet::~SubprocessSet() {}
+bool SubprocessSet::interrupted_;
+
+void SubprocessSet::SetInterruptedFlag(int signum) {
+ (void) signum;
+ interrupted_ = true;
+}
+
+SubprocessSet::SubprocessSet() {
+ interrupted_ = false;
+
+ sigset_t set;
+ sigemptyset(&set);
+ sigaddset(&set, SIGINT);
+ if (sigprocmask(SIG_BLOCK, &set, &old_mask_) < 0)
+ Fatal("sigprocmask: %s", strerror(errno));
+
+ struct sigaction act;
+ memset(&act, 0, sizeof(act));
+ act.sa_handler = SetInterruptedFlag;
+ if (sigaction(SIGINT, &act, &old_act_) < 0)
+ Fatal("sigaction: %s", strerror(errno));
+}
+
+SubprocessSet::~SubprocessSet() {
+ Clear();
-void SubprocessSet::Add(Subprocess* subprocess) {
+ if (sigaction(SIGINT, &old_act_, 0) < 0)
+ Fatal("sigaction: %s", strerror(errno));
+ if (sigprocmask(SIG_SETMASK, &old_mask_, 0) < 0)
+ Fatal("sigprocmask: %s", strerror(errno));
+}
+
+Subprocess *SubprocessSet::Add(const string &command) {
+ Subprocess *subprocess = new Subprocess;
+ if (!subprocess->Start(this, command)) {
+ delete subprocess;
+ return 0;
+ }
running_.push_back(subprocess);
+ return subprocess;
}
-void SubprocessSet::DoWork() {
- vector<pollfd> fds;
+bool SubprocessSet::DoWork() {
+ fd_set set;
+ int nfds = 0;
+ FD_ZERO(&set);
- map<int, Subprocess*> fd_to_subprocess;
for (vector<Subprocess*>::iterator i = running_.begin();
i != running_.end(); ++i) {
int fd = (*i)->fd_;
if (fd >= 0) {
- fd_to_subprocess[fd] = *i;
- fds.resize(fds.size() + 1);
- pollfd* newfd = &fds.back();
- newfd->fd = fd;
- newfd->events = POLLIN;
- newfd->revents = 0;
+ FD_SET(fd, &set);
+ if (nfds < fd+1)
+ nfds = fd+1;
}
}
- int ret = poll(&fds.front(), fds.size(), -1);
+ int ret = pselect(nfds, &set, 0, 0, 0, &old_mask_);
if (ret == -1) {
- if (errno != EINTR)
- perror("ninja: poll");
- return;
+ if (errno != EINTR) {
+ perror("ninja: pselect");
+ return false;
+ }
+ bool interrupted = interrupted_;
+ interrupted_ = false;
+ return interrupted;
}
- for (size_t i = 0; i < fds.size(); ++i) {
- if (fds[i].revents) {
- Subprocess* subproc = fd_to_subprocess[fds[i].fd];
- subproc->OnPipeReady();
- if (subproc->Done()) {
- finished_.push(subproc);
- std::remove(running_.begin(), running_.end(), subproc);
- running_.resize(running_.size() - 1);
+ for (vector<Subprocess*>::iterator i = running_.begin();
+ i != running_.end(); ) {
+ int fd = (*i)->fd_;
+ if (fd >= 0 && FD_ISSET(fd, &set)) {
+ (*i)->OnPipeReady();
+ if ((*i)->Done()) {
+ finished_.push(*i);
+ running_.erase(i);
+ continue;
}
}
+ ++i;
}
+
+ return false;
}
Subprocess* SubprocessSet::NextFinished() {
@@ -174,3 +230,13 @@ Subprocess* SubprocessSet::NextFinished() {
finished_.pop();
return subproc;
}
+
+void SubprocessSet::Clear() {
+ for (vector<Subprocess*>::iterator i = running_.begin();
+ i != running_.end(); ++i)
+ kill(-(*i)->pid_, SIGINT);
+ for (vector<Subprocess*>::iterator i = running_.begin();
+ i != running_.end(); ++i)
+ delete *i;
+ running_.clear();
+}
View
33 src/subprocess.h
@@ -22,25 +22,32 @@ using namespace std;
#ifdef _WIN32
#include <windows.h>
+#else
+#include <signal.h>
#endif
+#include "exit_status.h"
+
/// Subprocess wraps a single async subprocess. It is entirely
/// passive: it expects the caller to notify it when its fds are ready
/// for reading, as well as call Finish() to reap the child once done()
/// is true.
struct Subprocess {
- Subprocess();
~Subprocess();
- bool Start(struct SubprocessSet* set, const string& command);
- void OnPipeReady();
- /// Returns true on successful process exit.
- bool Finish();
+
+ /// Returns ExitSuccess on successful process exit, ExitInterrupted if
+ /// the process was interrupted, ExitFailure if it otherwise failed.
+ ExitStatus Finish();
bool Done() const;
const string& GetOutput() const;
private:
+ Subprocess();
+ bool Start(struct SubprocessSet* set, const string& command);
+ void OnPipeReady();
+
string buf_;
#ifdef _WIN32
@@ -60,22 +67,30 @@ struct Subprocess {
friend struct SubprocessSet;
};
-/// SubprocessSet runs a poll() loop around a set of Subprocesses.
+/// SubprocessSet runs a pselect() loop around a set of Subprocesses.
/// DoWork() waits for any state change in subprocesses; finished_
/// is a queue of subprocesses as they finish.
struct SubprocessSet {
SubprocessSet();
~SubprocessSet();
- void Add(Subprocess* subprocess);
- void DoWork();
+ Subprocess *Add(const string &command);
+ bool DoWork();
Subprocess* NextFinished();
+ void Clear();
vector<Subprocess*> running_;
queue<Subprocess*> finished_;
#ifdef _WIN32
- HANDLE ioport_;
+ static BOOL WINAPI NotifyInterrupted(DWORD dwCtrlType);
+ static HANDLE ioport_;
+#else
+ static void SetInterruptedFlag(int signum);
+ static bool interrupted_;
+
+ struct sigaction old_act_;
+ sigset_t old_mask_;
#endif
};
View
56 src/subprocess_test.cc
@@ -32,46 +32,71 @@ struct SubprocessTest : public testing::Test {
// Run a command that fails and emits to stderr.
TEST_F(SubprocessTest, BadCommandStderr) {
- Subprocess* subproc = new Subprocess;
- EXPECT_TRUE(subproc->Start(&subprocs_, "cmd /c ninja_no_such_command"));
- subprocs_.Add(subproc);
+ Subprocess* subproc = subprocs_.Add("cmd /c ninja_no_such_command");
+ ASSERT_NE((Subprocess *) 0, subproc);
while (!subproc->Done()) {
// Pretend we discovered that stderr was ready for writing.
subprocs_.DoWork();
}
- EXPECT_FALSE(subproc->Finish());
+ EXPECT_EQ(ExitFailure, subproc->Finish());
EXPECT_NE("", subproc->GetOutput());
}
// Run a command that does not exist
TEST_F(SubprocessTest, NoSuchCommand) {
- Subprocess* subproc = new Subprocess;
- EXPECT_TRUE(subproc->Start(&subprocs_, "ninja_no_such_command"));
- subprocs_.Add(subproc);
+ Subprocess* subproc = subprocs_.Add("ninja_no_such_command");
+ ASSERT_NE((Subprocess *) 0, subproc);
while (!subproc->Done()) {
// Pretend we discovered that stderr was ready for writing.
subprocs_.DoWork();
}
- EXPECT_FALSE(subproc->Finish());
+ EXPECT_EQ(ExitFailure, subproc->Finish());
EXPECT_NE("", subproc->GetOutput());
#ifdef _WIN32
ASSERT_EQ("CreateProcess failed: The system cannot find the file specified.\n", subproc->GetOutput());
#endif
}
+#ifndef _WIN32
+
+TEST_F(SubprocessTest, InterruptChild) {
+ Subprocess* subproc = subprocs_.Add("kill -INT $$");
+ ASSERT_NE((Subprocess *) 0, subproc);
+
+ while (!subproc->Done()) {
+ subprocs_.DoWork();
+ }
+
+ EXPECT_EQ(ExitInterrupted, subproc->Finish());
+}
+
+TEST_F(SubprocessTest, InterruptParent) {
+ Subprocess* subproc = subprocs_.Add("kill -INT $PPID ; sleep 1");
+ ASSERT_NE((Subprocess *) 0, subproc);
+
+ while (!subproc->Done()) {
+ bool interrupted = subprocs_.DoWork();
+ if (interrupted)
+ return;
+ }
+
+ ADD_FAILURE() << "We should have been interrupted";
+}
+
+#endif
+
TEST_F(SubprocessTest, SetWithSingle) {
- Subprocess* subproc = new Subprocess;
- EXPECT_TRUE(subproc->Start(&subprocs_, kSimpleCommand));
- subprocs_.Add(subproc);
+ Subprocess* subproc = subprocs_.Add(kSimpleCommand);
+ ASSERT_NE((Subprocess *) 0, subproc);
while (!subproc->Done()) {
subprocs_.DoWork();
}
- ASSERT_TRUE(subproc->Finish());
+ ASSERT_EQ(ExitSuccess, subproc->Finish());
ASSERT_NE("", subproc->GetOutput());
ASSERT_EQ(1u, subprocs_.finished_.size());
@@ -91,9 +116,8 @@ TEST_F(SubprocessTest, SetWithMulti) {
};
for (int i = 0; i < 3; ++i) {
- processes[i] = new Subprocess;
- EXPECT_TRUE(processes[i]->Start(&subprocs_, kCommands[i]));
- subprocs_.Add(processes[i]);
+ processes[i] = subprocs_.Add(kCommands[i]);
+ ASSERT_NE((Subprocess *) 0, processes[i]);
}
ASSERT_EQ(3u, subprocs_.running_.size());
@@ -112,7 +136,7 @@ TEST_F(SubprocessTest, SetWithMulti) {
ASSERT_EQ(3u, subprocs_.finished_.size());
for (int i = 0; i < 3; ++i) {
- ASSERT_TRUE(processes[i]->Finish());
+ ASSERT_EQ(ExitSuccess, processes[i]->Finish());
ASSERT_NE("", processes[i]->GetOutput());
delete processes[i];
}

0 comments on commit 85ff781

Please sign in to comment.