From: Adam Simpkins <simpkins@fb.com>
Date: Thu, 22 Jun 2017 03:38:47 +0000 (-0700)
Subject: logging: improve the AsyncFileWriter flush test()
X-Git-Tag: v2017.06.26.00~18
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=00ff5917775f9b58a04f74835585cbb32e306289;p=folly.git

logging: improve the AsyncFileWriter flush test()

Summary:
This test has run into occasional failures on continuous build test runs.
Unfortunately when something goes wrong it crashes in the std::thread
destructor due to this thread still being joinable when it is destroyed, which
hides information about what actually failed in the test.

This updates the test to immediately detach the thread, so that on error we
will be able see the real failure reason.

This also increases the size of the message that we write, which will hopefully
help ensure that this write always blocks.

Reviewed By: wez

Differential Revision: D5295574

fbshipit-source-id: ea8cfa855613398f88f9f982c600ec661018a31c
---

diff --git a/folly/experimental/logging/test/AsyncFileWriterTest.cpp b/folly/experimental/logging/test/AsyncFileWriterTest.cpp
index 7b03ef5e..3333c065 100644
--- a/folly/experimental/logging/test/AsyncFileWriterTest.cpp
+++ b/folly/experimental/logging/test/AsyncFileWriterTest.cpp
@@ -208,7 +208,7 @@ TEST(AsyncFileWriter, flush) {
   AsyncFileWriter writer{std::move(writePipe)};
 
   // Write a message
-  writer.writeMessage(std::string{"test message"});
+  writer.writeMessage("test message: " + std::string(200, 'x'));
 
   // Call flush().  Use a separate thread, since this should block until we
   // consume data from the pipe.
@@ -217,8 +217,16 @@ TEST(AsyncFileWriter, flush) {
   auto flushFunction = [&] { writer.flush(); };
   std::thread flushThread{
       [&]() { promise.setTry(makeTryWith(flushFunction)); }};
+  // Detach the flush thread now rather than joining it at the end of the
+  // function.  This way if something goes wrong during the test we will fail
+  // with the real error, rather than crashing due to the std::thread
+  // destructor running on a still-joinable thread.
+  flushThread.detach();
 
   // Sleep briefly, and make sure flush() still hasn't completed.
+  // If it has completed this doesn't necessarily indicate a bug in
+  // AsyncFileWriter, but instead indicates that our test code failed to
+  // successfully cause a blocking write.
   /* sleep override */
   std::this_thread::sleep_for(10ms);
   EXPECT_FALSE(future.isReady());
@@ -230,7 +238,6 @@ TEST(AsyncFileWriter, flush) {
 
   // Make sure flush completes successfully now
   future.get(10ms);
-  flushThread.join();
 }
 #endif