More tests.

test/gtest-extra-test.cc

@@ -372,12 +372,11 @@ TEST(FileTest, CloseError) {
   EXPECT_SYSTEM_ERROR(f.close(), EBADF, "cannot close file");
   EXPECT_EQ(-1, f.descriptor());
 #else
-  // Open other before closing f or the descriptor may be recycled.
+  File dup = f.dup(f.descriptor());
-  File other(".travis.yml", File::RDONLY);
   close(f.descriptor());
   // Closing file twice causes death on Windows.
   EXPECT_DEATH(f.close(), "");
-  other.dup2(f.descriptor());  // "undo" close or dtor will fail
+  dup.dup2(f.descriptor());  // "undo" close or dtor will fail
 #endif
 }
 
@@ -507,7 +506,49 @@ TEST(OutputRedirectTest, ScopedRedirect) {
   EXPECT_READ(read_end, "[[[]]]");
 }
 
-// TODO: test OutputRedirect
+// Test that OutputRedirect handles errors in flush correctly.
+TEST(OutputRedirectTest, ErrorInFlushBeforeRedirect) {
+  File read_end, write_end;
+  File::pipe(read_end, write_end);
+  int write_fd = write_end.descriptor();
+  File write_dup = write_end.dup(write_fd);
+  BufferedFile f = write_end.fdopen("w");
+  // Put a character in a file buffer.
+  EXPECT_EQ('x', fputc('x', f.get()));
+  close(write_fd);
+  OutputRedirect *redir = 0;
+  EXPECT_SYSTEM_ERROR_OR_DEATH(redir = new OutputRedirect(f.get()),
+      EBADF, fmt::Format("cannot flush stream"));
+  delete redir;
+  write_dup.dup2(write_fd);  // "undo" close or dtor will fail
+}
+
+TEST(OutputRedirectTest, DupError) {
+  BufferedFile f = OpenFile(".travis.yml");
+  int fd = fileno(f.get());
+  File dup = File::dup(fd);
+  close(fd);
+  OutputRedirect *redir = 0;
+  EXPECT_SYSTEM_ERROR_OR_DEATH(redir = new OutputRedirect(f.get()),
+      EBADF, fmt::Format("cannot duplicate file descriptor {}") << fd);
+  dup.dup2(fd);  // "undo" close or dtor will fail
+  delete redir;
+}
+
+TEST(OutputRedirectTest, RestoreAndRead) {
+  File read_end, write_end;
+  File::pipe(read_end, write_end);
+  BufferedFile file(write_end.fdopen("w"));
+  std::fprintf(file.get(), "[[[");
+  OutputRedirect redir(file.get());
+  std::fprintf(file.get(), "censored");
+  EXPECT_EQ("censored", redir.RestoreAndRead());
+  std::fprintf(file.get(), "]]]");
+  file = BufferedFile();
+  EXPECT_READ(read_end, "[[[]]]");
+}
+
+// TODO: test OutputRedirect - dtor error
 // TODO: test EXPECT_STDOUT and EXPECT_STDERR
 
 // TODO: compile both with C++11 & C++98 mode

test/gtest-extra.cc

@@ -163,16 +163,31 @@ BufferedFile File::fdopen(const char *mode) {
   return f;
 }
 
-OutputRedirect::OutputRedirect(FILE *file) : file_(file) {
+void OutputRedirect::Flush() {
-  if (std::fflush(file) != 0)
+#if EOF != -1
+# error "FMT_RETRY assumes return value of -1 indicating failure"
+#endif
+  int result = 0;
+  FMT_RETRY(result, fflush(file_));
+  if (result != 0)
     fmt::ThrowSystemError(errno, "cannot flush stream");
+}
+
+void OutputRedirect::Restore() {
+  Flush();
+  // Restore the original file.
+  original_.dup2(FMT_POSIX(fileno(file_)));
+}
+
+OutputRedirect::OutputRedirect(std::FILE *file) : file_(file) {
+  Flush();
   int fd = FMT_POSIX(fileno(file));
-  // Save the original file.
+  // Create a File object referring to 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.
+  // Connect the passed FILE object to the write end of the pipe.
   write_end.dup2(fd);
 }
 
@@ -184,14 +199,7 @@ OutputRedirect::~OutputRedirect() FMT_NOEXCEPT(true) {
   }
 }
 
-void OutputRedirect::Restore() {
+std::string OutputRedirect::RestoreAndRead() {
-  if (std::fflush(file_) != 0)
-    fmt::ThrowSystemError(errno, "cannot flush stream");
-  // Restore the original file.
-  original_.dup2(FMT_POSIX(fileno(file_)));
-}
-
-std::string OutputRedirect::Read() {
   // Restore output.
   Restore();
 

test/gtest-extra.h

@@ -29,6 +29,7 @@
 #define FMT_GTEST_EXTRA_H
 
 #include <cstddef>
+#include <cstdio>
 #include <ios>
 #include <string>
 
@@ -106,11 +107,11 @@ class ErrorCode {
 // A buffered file.
 class BufferedFile {
  private:
-  FILE *file_;
+  std::FILE *file_;
 
   friend class File;
 
-  explicit BufferedFile(FILE *f) : file_(f) {}
+  explicit BufferedFile(std::FILE *f) : file_(f) {}
 
   void close();
 
@@ -129,7 +130,7 @@ class BufferedFile {
   // A proxy object to emulate a move constructor.
   // It is private to make it impossible call operator Proxy directly.
   struct Proxy {
-    FILE *file;
+    std::FILE *file;
   };
 
  public:
@@ -180,7 +181,7 @@ class BufferedFile {
   }
 #endif
 
-  FILE *get() const { return file_; }
+  std::FILE *get() const { return file_; }
 };
 
 // A file.
@@ -299,6 +300,8 @@ class File {
   // and writing respectively.
   static void pipe(File &read_end, File &write_end);
 
+  // Creates a BufferedFile object associated with this file and detaches
+  // this File object from the file.
   BufferedFile fdopen(const char *mode);
 };
 
@@ -314,21 +317,22 @@ inline File &move(File &f) { return f; }
 // The output it can handle is limited by the pipe capacity.
 class OutputRedirect {
  private:
-  FILE *file_;
+  std::FILE *file_;
   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_(OutputRedirect);
 
+  void Flush();
   void Restore();
 
  public:
-  explicit OutputRedirect(FILE *file);
+  explicit OutputRedirect(std::FILE *file);
   ~OutputRedirect() FMT_NOEXCEPT(true);
 
   // Restores the original file, reads output from the pipe into a string
   // and returns it.
-  std::string Read();
+  std::string RestoreAndRead();
 };
 
 #define FMT_TEST_PRINT_(statement, expected_output, file, fail) \
@@ -338,7 +342,7 @@ class OutputRedirect {
     { \
       OutputRedirect redir(file); \
       GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \
-      output = redir.Read(); \
+      output = redir.RestoreAndRead(); \
     } \
     if (output != expected_output) { \
       gtest_ar \