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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 255914.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Assert on the execution order instead with simplifications.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77669

Files:
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp


Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -8,6 +8,7 @@
 
 #include "Annotations.h"
 #include "Cancellation.h"
+#include "ClangdServer.h"
 #include "Context.h"
 #include "Diagnostics.h"
 #include "Matchers.h"
@@ -844,41 +845,52 @@
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
-  ClangdServer Server(CDB, FS, ClangdServer::optsForTest(), &CaptureTUStatus);
+  auto Opts = ClangdServer::optsForTest();
+  ClangdServer Server(CDB, FS, Opts, &CaptureTUStatus);
   Annotations Code("int m^ain () {}");
 
   // We schedule the following tasks in the queue:
   //   [Update] [GoToDefinition]
   Server.addDocument(testPath("foo.cpp"), Code.code(), "1",
-                     WantDiagnostics::Yes);
+                     // Don't request diagnostics to simplify ASTActions by
+                     // getting rid of AST build.
+                     WantDiagnostics::No);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   Server.locateSymbolAt(testPath("foo.cpp"), Code.point(),
                         [](Expected<std::vector<LocatedSymbol>> Result) {
                           ASSERT_TRUE((bool)Result);
                         });
-
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
-  EXPECT_THAT(CaptureTUStatus.allStatus(),
-              ElementsAre(
-                  // Everything starts with ASTWorker starting to execute an
-                  // update
-                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-                  // We build the preamble
-                  TUState(PreambleAction::Building, ASTAction::RunningAction),
-                  // We built the preamble, and issued ast built on ASTWorker
-                  // thread. Preambleworker goes idle afterwards.
-                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-                  // Start task for building the ast, as a result of building
-                  // preamble, on astworker thread.
-                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-                  // AST build starts.
-                  TUState(PreambleAction::Idle, ASTAction::Building),
-                  // AST built finished successfully
-                  TUState(PreambleAction::Idle, ASTAction::Building),
-                  // Running go to def
-                  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-                  // ASTWorker goes idle.
-                  TUState(PreambleAction::Idle, ASTAction::Idle)));
+  EXPECT_THAT(
+      CaptureTUStatus.allStatus(),
+      ElementsAre(
+          // Everything starts with ASTWorker starting to execute an update.
+          TUState(PreambleAction::Idle, ASTAction::RunningAction),
+          // PreambleThread starts working, ASTWorker can't go idle as it blocks
+          // for first preamble build.
+          TUState(PreambleAction::Building, ASTAction::RunningAction),
+          // Now preamble is build and ASTWorker is unblocked. Either:
+          // - ASTWorker finishes update and starts handling preamble
+          // - PreambleThread goes idle.
+          AnyOf(TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                TUState(PreambleAction::Building, ASTAction::RunningAction)),
+          // If PreambleThread went idle in previous case:
+          // - ASTWorker finishes update and starts handling preamble
+          // If PreambleThread was still building:
+          // - It will go idle, ASTWorker will stays the same
+          // - Before it can go idle, ASTWorker finish handling preamble and go
+          //   idle.
+          AnyOf(TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                TUState(PreambleAction::Idle, ASTAction::RunningAction),
+                TUState(PreambleAction::Building, ASTAction::Idle)),
+          // Finally both workers go idle.
+          TUState(PreambleAction::Idle, ASTAction::Idle),
+          // Now we start handling GoToDefinition, ASTWorker starts running
+          // action.
+          TUState(PreambleAction::Idle, ASTAction::RunningAction),
+          // Builds AST and serves the request, then goes idle.
+          TUState(PreambleAction::Idle, ASTAction::Idle)));
 }
 
 TEST_F(TUSchedulerTests, CommandLineErrors) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77669.255914.patch
Type: text/x-patch
Size: 4461 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200408/81ef3968/attachment.bin>