sammccall added inline comments.
================
Comment at: unittests/clangd/ClangdTests.cpp:1037
+TEST(ClangdTests, AddDocumentTracksDiagnostics) {
+ // We have clients relying on the fact that the Context passed to addDocument
----------------
this tests a lot of things, and after 10 minutes I can't understand exactly
what.
AIUI the thing we really want to verify is that if `TUScheduler::update`
decides not to update, then context death is a timely signal that it did so.
(Doing this test at the ClangdServer level makes it more black-box, but I don't
think that justifies the complexity.)
So isn't it enough to schedule 3 updates, make sure the middle one does no
work, and verify its context is destroyed in between the other two? e.g.
```
atomic<int> Counter(0);
TUScheduler S;
Notification Start;
S.update("a.cc", inputs1, WantDiagnostics::Yes, []{
// ensure we finish scheduling before everything finishes, else
// we're not really testing anything
Start.wait();
EXPECT_EQ(1, ++Counter);
});
{
WithContextValue WatchDestructor(llvm::make_scope_exit() {
EXPECT_EQ(2, ++Counter);
});
S.update("a.cc", inputs1, WantDiagnostics::Yes, []{
FAIL() << "Expect no diagnostics, inputs unchanged";
});
}
S.update("a.cc", inputs2, WantDiagnostics::Yes, []{
EXPECT_EQ(3, ++Counter);
});
EXPECT_EQ(0, Counter);
Start.notify();
S.blockUntilIdle();
EXPECT_EQ(3, Counter);
```
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53946
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
