kadircet created this revision.
kadircet added reviewers: sammccall, nridge.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
We seem to be hitting limits in some windows build bots, see
https://github.com/clangd/clangd/issues/1712#issuecomment-1686478931.
So bumping the timeouts to 60 seconds and completely dropping them for sync
requests. As mentioned in the comment above, this should improve things,
considering even the tests that don't touch any complicated scheduler is
failing.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158426
Files:
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
clang-tools-extra/clangd/unittests/LSPClient.cpp
clang-tools-extra/clangd/unittests/LSPClient.h
Index: clang-tools-extra/clangd/unittests/LSPClient.h
===================================================================
--- clang-tools-extra/clangd/unittests/LSPClient.h
+++ clang-tools-extra/clangd/unittests/LSPClient.h
@@ -9,12 +9,14 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+#include "llvm/ADT/StringRef.h"
+#include <condition_variable>
#include <llvm/Support/Error.h>
#include <llvm/Support/JSON.h>
-#include <condition_variable>
-#include <deque>
+#include <memory>
#include <mutex>
#include <optional>
+#include <vector>
namespace clang {
namespace clangd {
@@ -32,11 +34,12 @@
class CallResult {
public:
~CallResult();
- // Blocks up to 10 seconds for the result to be ready.
+ // Blocks up to \p TimeoutSeconds seconds for the result to be ready. If \p
+ // TimeoutSeconds is zero, blocks indefinitely.
// Records a test failure if there was no reply.
- llvm::Expected<llvm::json::Value> take();
+ llvm::Expected<llvm::json::Value> take(float TimeoutSeconds = 60);
// Like take(), but records a test failure if the result was an error.
- llvm::json::Value takeValue();
+ llvm::json::Value takeValue(float TimeoutSeconds = 60);
private:
// Should be called once to provide the value.
Index: clang-tools-extra/clangd/unittests/LSPClient.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/LSPClient.cpp
+++ clang-tools-extra/clangd/unittests/LSPClient.cpp
@@ -12,19 +12,36 @@
#include "Transport.h"
#include "support/Logger.h"
#include "support/Threading.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/JSON.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include "gtest/gtest.h"
#include <condition_variable>
+#include <cstdint>
+#include <deque>
+#include <functional>
+#include <memory>
+#include <mutex>
#include <optional>
#include <queue>
+#include <string>
+#include <utility>
+#include <vector>
namespace clang {
namespace clangd {
-llvm::Expected<llvm::json::Value> clang::clangd::LSPClient::CallResult::take() {
+llvm::Expected<llvm::json::Value>
+clang::clangd::LSPClient::CallResult::take(float TimeoutSeconds) {
std::unique_lock<std::mutex> Lock(Mu);
- if (!clangd::wait(Lock, CV, timeoutSeconds(10),
+ if (!clangd::wait(Lock, CV,
+ timeoutSeconds(TimeoutSeconds
+ ? std::optional<float>(TimeoutSeconds)
+ : std::nullopt),
[this] { return Value.has_value(); })) {
ADD_FAILURE() << "No result from call after 10 seconds!";
return llvm::json::Value(nullptr);
@@ -34,8 +51,8 @@
return Res;
}
-llvm::json::Value LSPClient::CallResult::takeValue() {
- auto ExpValue = take();
+llvm::json::Value LSPClient::CallResult::takeValue(float TimeoutSeconds) {
+ auto ExpValue = take(TimeoutSeconds);
if (!ExpValue) {
ADD_FAILURE() << "takeValue(): " << llvm::toString(ExpValue.takeError());
return llvm::json::Value(nullptr);
@@ -197,7 +214,10 @@
notify("textDocument/didClose", Obj{{"textDocument", documentID(Path)}});
}
-void LSPClient::sync() { call("sync", nullptr).takeValue(); }
+void LSPClient::sync() {
+ // Sync should already be implemented with a timeout, so don't timeout.
+ call("sync", nullptr).takeValue(0);
+}
std::optional<std::vector<llvm::json::Value>>
LSPClient::diagnostics(llvm::StringRef Path) {
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -8,19 +8,38 @@
#include "Annotations.h"
#include "ClangdLSPServer.h"
+#include "ClangdServer.h"
+#include "ConfigProvider.h"
+#include "Diagnostics.h"
+#include "FeatureModule.h"
+#include "LSPBinder.h"
#include "LSPClient.h"
-#include "Protocol.h"
#include "TestFS.h"
+#include "support/Function.h"
#include "support/Logger.h"
#include "support/TestTracer.h"
+#include "support/Threading.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/JSON.h"
+#include "llvm/Support/raw_ostream.h"
#include "llvm/Testing/Support/Error.h"
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <cassert>
+#include <condition_variable>
+#include <cstddef>
+#include <deque>
+#include <memory>
+#include <mutex>
#include <optional>
+#include <thread>
+#include <utility>
namespace clang {
namespace clangd {
@@ -358,7 +377,7 @@
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
- EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
+ Client.sync();
EXPECT_EQ(3, FeatureModules.get<AsyncCounter>()->getSync());
// Throw some work on the queue to make sure shutdown blocks on it.
Client.notify("increment", nullptr);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
