[PATCH] D77669: [clangd] Update TUStatus to handle async PreambleThread

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:849
+  auto Opts = ClangdServer::optsForTest();
+  Opts.AsyncThreadsCount = 0;
+  ClangdServer Server(CDB, FS, Opts, &CaptureTUStatus);
----------------
sammccall wrote:
> This seems unfortunate because it doesn't test the production configuration.
> 
> How many possible sequences are there? Are there fewer if we blockuntilidle between adddoc + locatesymbolat?
> 
> Can we use testing::AnyOf to fudge around the nondeterminism?
I didn't want to do that as it was rather asserting on action ordering of preamble and ast worker threads rather than checking for whether we emit TUStatuses, but I suppose this test was always trying to assert both.

Introducing some more simplications by turning of diagnostics and also making use of the assumption that ASTWorker will block for first preamble build.


================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:869
+                  TUState(PreambleAction::Building, ASTAction::Building),
+                  TUState(PreambleAction::Idle, ASTAction::Building)));
 }
----------------
sammccall wrote:
> why do we never go idle at the end?
because astworker doesn't emit queue related notifications(running action, queued, idle) in RunSync mode.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77669/new/

https://reviews.llvm.org/D77669