diff --git a/test/gtest-extra-test.cc b/test/gtest-extra-test.cc index 79254c73..7cfafbd5 100644 --- a/test/gtest-extra-test.cc +++ b/test/gtest-extra-test.cc @@ -243,7 +243,7 @@ TEST(BufferedFileTest, CloseFileInDtor) { EXPECT_CLOSED(fd); } -TEST(BufferedFileTest, DtorCloseError) { +TEST(BufferedFileTest, CloseErrorInDtor) { BufferedFile *f = new BufferedFile(OpenFile(".travis.yml")); #ifndef _WIN32 // The close function must be called inside EXPECT_STDERR, otherwise @@ -337,7 +337,7 @@ TEST(FileTest, CloseFileInDtor) { EXPECT_CLOSED(fd); } -TEST(FileTest, DtorCloseError) { +TEST(FileTest, CloseErrorInDtor) { File *f = new File(".travis.yml", File::RDONLY); #ifndef _WIN32 // The close function must be called inside EXPECT_STDERR, otherwise @@ -507,7 +507,7 @@ TEST(OutputRedirectTest, ScopedRedirect) { } // Test that OutputRedirect handles errors in flush correctly. -TEST(OutputRedirectTest, ErrorInFlushBeforeRedirect) { +TEST(OutputRedirectTest, FlushErrorInCtor) { File read_end, write_end; File::pipe(read_end, write_end); int write_fd = write_end.descriptor(); @@ -523,7 +523,7 @@ TEST(OutputRedirectTest, ErrorInFlushBeforeRedirect) { write_dup.dup2(write_fd); // "undo" close or dtor will fail } -TEST(OutputRedirectTest, DupError) { +TEST(OutputRedirectTest, DupErrorInCtor) { BufferedFile f = OpenFile(".travis.yml"); int fd = fileno(f.get()); File dup = File::dup(fd); @@ -548,7 +548,42 @@ TEST(OutputRedirectTest, RestoreAndRead) { EXPECT_READ(read_end, "[[[]]]"); } -// TODO: test OutputRedirect - dtor error +// Test that OutputRedirect handles errors in flush correctly. +TEST(OutputRedirectTest, FlushErrorInRestoreAndRead) { + 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"); + OutputRedirect redir(f.get()); + // Put a character in a file buffer. + EXPECT_EQ('x', fputc('x', f.get())); + close(write_fd); + EXPECT_SYSTEM_ERROR_OR_DEATH(redir.RestoreAndRead(), + EBADF, fmt::Format("cannot flush stream")); + write_dup.dup2(write_fd); // "undo" close or dtor will fail +} + +TEST(OutputRedirectTest, ErrorInDtor) { + 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"); + OutputRedirect *redir = new OutputRedirect(f.get()); + // Put a character in a file buffer. + EXPECT_EQ('x', fputc('x', f.get())); + // 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(write_fd); delete redir, + FormatSystemErrorMessage(EBADF, "cannot flush stream")); + write_dup.dup2(write_fd); // "undo" close or dtor of BufferedFile will fail +} + +// TODO: test calling RestoreAndRead multiple times + // TODO: test EXPECT_STDOUT and EXPECT_STDERR // TODO: compile both with C++11 & C++98 mode diff --git a/test/gtest-extra.cc b/test/gtest-extra.cc index 513c590f..503a9cb4 100644 --- a/test/gtest-extra.cc +++ b/test/gtest-extra.cc @@ -174,9 +174,12 @@ void OutputRedirect::Flush() { } void OutputRedirect::Restore() { + if (original_.descriptor() == -1) + return; // Already restored. Flush(); // Restore the original file. original_.dup2(FMT_POSIX(fileno(file_))); + original_.close(); } OutputRedirect::OutputRedirect(std::FILE *file) : file_(file) { @@ -195,7 +198,7 @@ OutputRedirect::~OutputRedirect() FMT_NOEXCEPT(true) { try { Restore(); } catch (const std::exception &e) { - // TODO: report + std::fputs(e.what(), stderr); } } @@ -205,6 +208,8 @@ std::string OutputRedirect::RestoreAndRead() { // Read everything from the pipe. std::string content; + if (read_end_.descriptor() == -1) + return content; // Already read. enum { BUFFER_SIZE = 4096 }; char buffer[BUFFER_SIZE]; std::streamsize count = 0; @@ -212,6 +217,7 @@ std::string OutputRedirect::RestoreAndRead() { count = read_end_.read(buffer, BUFFER_SIZE); content.append(buffer, static_cast(count)); } while (count != 0); + read_end_.close(); return content; } diff --git a/test/gtest-extra.h b/test/gtest-extra.h index 57b81f2e..5016f3bc 100644 --- a/test/gtest-extra.h +++ b/test/gtest-extra.h @@ -338,17 +338,17 @@ class OutputRedirect { #define FMT_TEST_PRINT_(statement, expected_output, file, fail) \ GTEST_AMBIGUOUS_ELSE_BLOCKER_ \ if (::testing::AssertionResult gtest_ar = ::testing::AssertionSuccess()) { \ - std::string output; \ + std::string gtest_output; \ { \ - OutputRedirect redir(file); \ + OutputRedirect gtest_redir(file); \ GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \ - output = redir.RestoreAndRead(); \ + gtest_output = gtest_redir.RestoreAndRead(); \ } \ - if (output != expected_output) { \ + if (gtest_output != expected_output) { \ gtest_ar \ << #statement " produces different output.\n" \ << "Expected: " << expected_output << "\n" \ - << " Actual: " << output; \ + << " Actual: " << gtest_output; \ goto GTEST_CONCAT_TOKEN_(gtest_label_testthrow_, __LINE__); \ } \ } else \