[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: jkorous, arphaman, akyrtzi, gribozavr.
smeenai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We've observed this test being significantly flaky on our Mac CI
machines when we're running the full check-clang suite. It fails because
the wait_for condition isn't met within 3 seconds. We believe it's
because our CI machines are somewhat underpowered and pretty heavily
loaded when we're running the full check-clang suite.

I ran some experiments on increasing the timeout. I ran the full
check-clang suite 100 times with each timeout value and recorded how
many flaky failures we encountered in these tests. The results are:

3 second timeout (baseline): 20 failures
10 second timeout: 14 failures
20 second timeout: 4 failures
30 second timeout: 2 failures
40 second timeout: 1 failure
50 second timeout: 0 failures
60 second timeout: 0 failures

I ran another set of 100 tests for the 50 second timeout and observed
one flaky failure. By contrast, I ended up running check-clang 500 times
for the 60 second timeout and didn't observe a single flaky failure.
That's how the 60 second timeout value used in this patch was derived.

While a 60 second timeout might seem high, keep in mind that:

- This is a timeout, not a sleep; the test should require much less time the vast majority of instances, especially on more powerful machines.
- The long timeout is most likely to occur when other tests are also running at the same time, so the latency of the timeout will also be masked by the latency of the other tests.

See https://reviews.llvm.org/D58418?id=200123#inline-554211 for where
this timeout was originally introduced and the possibility of raising it
if it wasn't enough was discussed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97878

Files:
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp


Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===================================================================
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -243,7 +243,7 @@
   std::thread worker(std::move(task));
   worker.detach();
 
-  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(60)) ==
               std::future_status::ready)
       << "The expected result state wasn't reached before the time-out.";
   std::unique_lock<std::mutex> L(TestConsumer.Mtx);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97878.327874.patch
Type: text/x-patch
Size: 668 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210303/ac34dd6f/attachment.bin>