jkorous marked 6 inline comments as done.
jkorous added inline comments.
================
Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:131
+ // the async event handling picks them up. Can make this test flaky.
+ std::this_thread::sleep_for(std::chrono::milliseconds(100)); // 0.1s
+}
----------------
jkorous wrote:
> gribozavr wrote:
> > I'm certain this sleep will be flaky on heavily-loaded CI machines. If you
> > are going to leave it as a sleep, please make it 1s. But is there really
> > no better way?
> That was exactly my thinking! Honestly, I don't know - I wasn't able to come
> up with any reasonably simple, deterministic approach even on a single
> platform :(
> I eventually picked 0.1s as a tradeoff between slowing the test for everyone
> and getting less false positives.
>
> The problem as I understand it is that we're making changes and monitoring
> them asynchronously with no guarantee from the kernel API (true for FSEvents,
> not 100% about inotify) about when (if) we receive notifications.
>
> If you have any idea for robust testing approach I'd be totally happy to use
> it.
I found a way how to use more generous timeout without slowing down the test
for every run - inspired by Argyrios' idea about using different thread +
semaphore.
I am using 3 seconds for now. If that's not enough, just let me know.
================
Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:357
+
+ EXPECT_TRUE(eventConsumer.AreExpectedPresentInNonInitial(
+ {{DirectoryWatcher::Event::EventKind::WatcherGotInvalidated, ""}}));
----------------
gribozavr wrote:
> I would strongly prefer if you used the gmock matchers (like Contains); as
> written, when the test fails, the only error we would get would be like
> "expected: true, actual: false".
Since the tests are using are based on something like "eventual correctness"
instead of one-time check I didn't use gmock matchers but implemented some
custom diagnostics.
Example of the failed test:
```
/Users/jankorous/src/llvm-monorepo/llvm-project/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:185:
Failure
Value of: *TestConsumer.Result()
Actual: false
Expected: true
Expected but not seen non-initial events:
Removed a
Unexpected non-initial events seen:
Added a
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58418/new/
https://reviews.llvm.org/D58418
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
