smeenai updated this revision to Diff 328283.
smeenai added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97878/new/
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
@@ -34,6 +34,17 @@
typedef DirectoryWatcher::Event::EventKind EventKind;
+// We've observed this test being significantly flaky when running on a heavily
+// loaded machine (e.g. when it's being run as part of the full check-clang
+// suite). Set a high timeout value to avoid this flakiness. The 60s timeout
+// value was determined empirically. It's a timeout value, not a sleep value,
+// and the test should require much less time in practice the vast majority of
+// instances. The cases where we do come close to (or still end up hitting) the
+// longer timeout are most likely to occur when other tests are also running at
+// the same time (e.g. as part of the full check-clang suite), in which case
the
+// latency of the timeout will be masked by the latency of the other tests.
+constexpr std::chrono::seconds EventualResultTimeout(60);
+
struct DirectoryWatcherTestFixture {
std::string TestRootDir;
std::string TestWatchedDir;
@@ -243,7 +254,7 @@
std::thread worker(std::move(task));
worker.detach();
- EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+ EXPECT_TRUE(WaitForExpectedStateResult.wait_for(EventualResultTimeout) ==
std::future_status::ready)
<< "The expected result state wasn't reached before the time-out.";
std::unique_lock<std::mutex> L(TestConsumer.Mtx);
Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===================================================================
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -34,6 +34,17 @@
typedef DirectoryWatcher::Event::EventKind EventKind;
+// We've observed this test being significantly flaky when running on a heavily
+// loaded machine (e.g. when it's being run as part of the full check-clang
+// suite). Set a high timeout value to avoid this flakiness. The 60s timeout
+// value was determined empirically. It's a timeout value, not a sleep value,
+// and the test should require much less time in practice the vast majority of
+// instances. The cases where we do come close to (or still end up hitting) the
+// longer timeout are most likely to occur when other tests are also running at
+// the same time (e.g. as part of the full check-clang suite), in which case the
+// latency of the timeout will be masked by the latency of the other tests.
+constexpr std::chrono::seconds EventualResultTimeout(60);
+
struct DirectoryWatcherTestFixture {
std::string TestRootDir;
std::string TestWatchedDir;
@@ -243,7 +254,7 @@
std::thread worker(std::move(task));
worker.detach();
- EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+ EXPECT_TRUE(WaitForExpectedStateResult.wait_for(EventualResultTimeout) ==
std::future_status::ready)
<< "The expected result state wasn't reached before the time-out.";
std::unique_lock<std::mutex> L(TestConsumer.Mtx);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
