[PATCH] D77669: [clangd] Run TUStatus test in sync mode to make output deterministic

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 7 18:32:39 PDT 2020


sammccall 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);
----------------
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?


================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:866
+      CaptureTUStatus.allStatus(),
+      ElementsAre(TUState(PreambleAction::Building, ASTAction::Idle),
+                  TUState(PreambleAction::Building, ASTAction::Building),
----------------
FWIW, I found the comments useful, and don't understand e.g. why we have two identical lines anymore.


================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:869
+                  TUState(PreambleAction::Building, ASTAction::Building),
+                  TUState(PreambleAction::Idle, ASTAction::Building)));
 }
----------------
why do we never go idle at the end?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77669





More information about the cfe-commits mailing list