Fix test. More comments.

test/gtest-extra-test.cc

@@ -248,8 +248,9 @@ TEST(FileTest, DtorCloseError) {
   File *f = new File(".travis.yml", File::RDONLY);
 #ifndef _WIN32
   // The close function must be called inside EXPECT_STDERR, otherwise
-  // the system may allocate freed file descriptor when redirecting the
+  // the system may recycle closed file descriptor when redirecting the
-  // output in EXPECT_STDERR.
+  // output in EXPECT_STDERR and the second close will break output
+  // redirection.
   EXPECT_STDERR(FMT_POSIX(close(f->descriptor())); delete f,
     FormatSystemErrorMessage(EBADF, "cannot close file") + "\n");
 #else
@@ -269,14 +270,25 @@ TEST(FileTest, Close) {
 
 TEST(FileTest, CloseError) {
   File *f = new File(".travis.yml", File::RDONLY);
+#ifndef _WIN32
   fmt::SystemError error("", 0);
   std::string message = FormatSystemErrorMessage(EBADF, "cannot close file");
+  // The close function must be called inside EXPECT_STDERR, otherwise
+  // the system may recycle closed file descriptor when redirecting the
+  // output in EXPECT_STDERR and the second close will break output
+  // redirection.
   EXPECT_STDERR(
     close(f->descriptor());
     try { f->close(); } catch (const fmt::SystemError &e) { error = e; }
     delete f,
     message + "\n");
   EXPECT_EQ(message, error.what());
+#else
+  close(f->descriptor());
+  // Closing file twice causes death on Windows.
+  f->close();
+  delete f;
+#endif
 }
 
 // Attempts to read count characters from a file.
@@ -382,11 +394,10 @@ TEST(FileTest, Pipe) {
   EXPECT_READ(read_end, "test");
 }
 
-// TODO: test pipe
-
 // TODO: compile both with C++11 & C++98 mode
 #endif
 
 // TODO: test OutputRedirector
+// TODO: test EXPECT_STDOUT and EXPECT_STDERR
 
 }  // namespace

test/gtest-extra.cc

@@ -146,26 +146,33 @@ OutputRedirector::OutputRedirector(FILE *file) : file_(file) {
   if (std::fflush(file) != 0)
     fmt::ThrowSystemError(errno, "cannot flush stream");
   int fd = FMT_POSIX(fileno(file));
-  saved_ = File::dup(fd);
+  // Save the original file.
+  original_ = File::dup(fd);
+  // Create a pipe.
   File write_end;
   File::pipe(read_end_, write_end);
+  // Connect the write end to the passed FILE object.
   write_end.dup2(fd);
 }
 
-OutputRedirector::~OutputRedirector() {
+OutputRedirector::~OutputRedirector() FMT_NOEXCEPT(true) {
+  try {
+    Restore();
+  } catch (const std::exception &e) {
+    // TODO: report
+  }
+}
+
+void OutputRedirector::Restore() {
   if (std::fflush(file_) != 0)
-    fmt::ReportSystemError(errno, "cannot flush stream");
+    fmt::ThrowSystemError(errno, "cannot flush stream");
-  ErrorCode ec;
+  // Restore the original file.
-  saved_.dup2(FMT_POSIX(fileno(file_)), ec);
+  original_.dup2(FMT_POSIX(fileno(file_)));
-  if (ec.get())
-    fmt::ReportSystemError(errno, "cannot restore output");
 }
 
 std::string OutputRedirector::Read() {
   // Restore output.
-  if (std::fflush(file_) != 0)
+  Restore();
-    fmt::ThrowSystemError(errno, "cannot flush stream");
-  saved_.dup2(FMT_POSIX(fileno(file_)));
 
   // Read everything from the pipe.
   std::string content;
@@ -179,6 +186,4 @@ std::string OutputRedirector::Read() {
   return content;
 }
 
-// TODO: test EXPECT_STDOUT and EXPECT_STDERR
-
 #endif  // FMT_USE_FILE_DESCRIPTORS

test/gtest-extra.h

@@ -224,19 +224,24 @@ inline File &move(File &f) { return f; }
 }
 #endif
 
-// Redirect file output to a pipe.
+// Captures file output by redirecting it to a pipe.
+// The output it can handle is limited by the pipe capacity.
 class OutputRedirector {
  private:
   FILE *file_;
-  File saved_;     // Saved file created with dup.
+  File original_;  // Original file passed to redirector.
   File read_end_;  // Read end of the pipe where the output is redirected.
 
   GTEST_DISALLOW_COPY_AND_ASSIGN_(OutputRedirector);
 
+  void Restore();
+
  public:
   explicit OutputRedirector(FILE *file);
-  ~OutputRedirector();
+  ~OutputRedirector() FMT_NOEXCEPT(true);
 
+  // Restores the original file, reads output from the pipe into a string
+  // and returns it.
   std::string Read();
 };