[clang-tools-extra] 48d851a - [clangd] Update TUStatus test to handle async PreambleThread

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-04-13T12:27:50+02:00
New Revision: 48d851a92e98ca9c425d4d9bc43def3693895498

URL: https://github.com/llvm/llvm-project/commit/48d851a92e98ca9c425d4d9bc43def3693895498
DIFF: https://github.com/llvm/llvm-project/commit/48d851a92e98ca9c425d4d9bc43def3693895498.diff

LOG: [clangd] Update TUStatus test to handle async PreambleThread

Summary:
Currently it doesn't matter because we run PreambleThread in sync mode.
Once we start running it in async, test order will become non-deterministic.

Reviewers: sammccall

Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, mgrang, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77669

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 2a0e6b001c00..bacd1f6b5d98 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/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"
@@ -829,18 +830,35 @@ TEST_F(TUSchedulerTests, TUStatus) {
   class CaptureTUStatus : public ClangdServer::Callbacks {
   public:
     void onFileUpdated(PathRef File, const TUStatus &Status) override {
+      auto ASTAction = Status.ASTActivity.K;
+      auto PreambleAction = Status.PreambleActivity;
       std::lock_guard<std::mutex> Lock(Mutex);
-      AllStatus.push_back(Status);
+      // Only push the action if it has changed. Since TUStatus can be published
+      // from either Preamble or AST thread and when one changes the other stays
+      // the same.
+      // Note that this can result in missing some updates when something other
+      // than action kind changes, e.g. when AST is built/reused the action kind
+      // stays as Building.
+      if (ASTActions.empty() || ASTActions.back() != ASTAction)
+        ASTActions.push_back(ASTAction);
+      if (PreambleActions.empty() || PreambleActions.back() != PreambleAction)
+        PreambleActions.push_back(PreambleAction);
     }
 
-    std::vector<TUStatus> allStatus() {
+    std::vector<PreambleAction> preambleStatuses() {
       std::lock_guard<std::mutex> Lock(Mutex);
-      return AllStatus;
+      return PreambleActions;
+    }
+
+    std::vector<ASTAction::Kind> astStatuses() {
+      std::lock_guard<std::mutex> Lock(Mutex);
+      return ASTActions;
     }
 
   private:
     std::mutex Mutex;
-    std::vector<TUStatus> AllStatus;
+    std::vector<ASTAction::Kind> ASTActions;
+    std::vector<PreambleAction> PreambleActions;
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
@@ -850,35 +868,39 @@ TEST_F(TUSchedulerTests, TUStatus) {
   // We schedule the following tasks in the queue:
   //   [Update] [GoToDefinition]
   Server.addDocument(testPath("foo.cpp"), Code.code(), "1",
-                     WantDiagnostics::Yes);
+                     WantDiagnostics::Auto);
+  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(),
+  EXPECT_THAT(CaptureTUStatus.preambleStatuses(),
+              ElementsAre(
+                  // PreambleThread starts idle, as the update is first handled
+                  // by ASTWorker.
+                  PreambleAction::Idle,
+                  // Then it starts building first preamble and releases that to
+                  // ASTWorker.
+                  PreambleAction::Building,
+                  // Then goes idle and stays that way as we don't receive any
+                  // more update requests.
+                  PreambleAction::Idle));
+  EXPECT_THAT(CaptureTUStatus.astStatuses(),
               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)));
+                  // Starts handling the update action and blocks until the
+                  // first preamble is built.
+                  ASTAction::RunningAction,
+                  // Afterwqards it builds an AST for that preamble to publish
+                  // diagnostics.
+                  ASTAction::Building,
+                  // Then goes idle.
+                  ASTAction::Idle,
+                  // Afterwards we start executing go-to-def.
+                  ASTAction::RunningAction,
+                  // Then go idle.
+                  ASTAction::Idle));
 }
 
 TEST_F(TUSchedulerTests, CommandLineErrors) {