[PATCH] D53946: [clangd] Test Context can be used for file status. NFC

[Sam McCall via Phabricator via cfe-commits]

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