From ee9aa75462460bf355a9744bd49dd9a619f504fc Mon Sep 17 00:00:00 2001 From: Chris Simon Date: Sun, 26 Nov 2023 22:02:58 +0000 Subject: [PATCH] test: improve language-server test resilience primarily by increasing timeouts, ensuring testclient and testlanguageserver is disposed properly, and adding missing conditionawaiters. Likely still a testclient memory leak - see https://github.com/OmniSharp/csharp-language-server-protocol/issues/1037 --- .../ConfigurationTests.fs | 7 ++- .../DefinitionsInitializationTests.fs | 10 ++--- .../DefinitionsTests.fs | 2 +- .../Helpers/ConditionAwaiter.fs | 21 +++++++-- .../Helpers/ServerLog.fs | 2 +- .../Helpers/TestClient.fs | 13 ++++-- .../InitializationTests.fs | 4 ++ .../Program.fs | 1 + .../WatchedFilesTests.fs | 14 +++--- .../fixtures/completion_tests/three.yml | 43 ++++++++++++++++++- 10 files changed, 93 insertions(+), 24 deletions(-) diff --git a/src/language-server/Contextive.LanguageServer.Tests/ConfigurationTests.fs b/src/language-server/Contextive.LanguageServer.Tests/ConfigurationTests.fs index bbdc490b..47b2e8fd 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/ConfigurationTests.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/ConfigurationTests.fs @@ -27,15 +27,20 @@ let tests = let pathLoader () : obj = path + let loadingAwaiter = ConditionAwaiter.create () + let config = [ Workspace.optionsBuilder <| Path.Combine("fixtures", "completion_tests") - ConfigurationSection.contextivePathLoaderBuilder pathLoader ] + ConfigurationSection.contextivePathLoaderBuilder pathLoader + ServerLog.optionsBuilder loadingAwaiter ] use! client = TestClient(config) |> init path <- "two.yml" ConfigurationSection.didChangePath client path + do! ServerLog.waitForLogMessage loadingAwaiter "Loading contextive" |> Async.Ignore + let! labels = Completion.getCompletionLabels client test <@ (labels, Fixtures.Two.expectedCompletionLabels) ||> Seq.compareWith compare = 0 @> diff --git a/src/language-server/Contextive.LanguageServer.Tests/DefinitionsInitializationTests.fs b/src/language-server/Contextive.LanguageServer.Tests/DefinitionsInitializationTests.fs index 24c22bc1..374f17c9 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/DefinitionsInitializationTests.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/DefinitionsInitializationTests.fs @@ -25,7 +25,7 @@ let tests = [ Workspace.optionsBuilder "" ConfigurationSection.contextivePathBuilder pathValue ] - let! (client, _) = TestClient(config) |> initAndWaitForReply + use! client = TestClient(config) |> init let responseRouterReturns = client.SendRequest("contextive/initialize") @@ -59,7 +59,7 @@ let tests = ConfigurationSection.contextivePathBuilder pathValue showDocumentRequestHandlerBuilder <| handler showDocAwaiter showDocResponse ] - let! (client, _) = TestClient(config) |> initAndWaitForReply + use! client = TestClient(config) |> init let responseRouterReturns = client.SendRequest("contextive/initialize") @@ -70,7 +70,7 @@ let tests = File.Delete(pathValue) - let! showDocMsg = ConditionAwaiter.waitForAny showDocAwaiter 5000 + let! showDocMsg = ConditionAwaiter.waitForAny showDocAwaiter test <@ showDocMsg.Value.Uri.ToString().Contains(pathValue) @> } @@ -90,7 +90,7 @@ let tests = let existingContents = File.ReadAllText(fullPath) - let! (client, _) = TestClient(config) |> initAndWaitForReply + use! client = TestClient(config) |> init let responseRouterReturns = client.SendRequest("contextive/initialize") @@ -99,7 +99,7 @@ let tests = |> Async.AwaitTask |> Async.Ignore - let! showDocMsg = ConditionAwaiter.waitForAny showDocAwaiter 3000 + let! showDocMsg = ConditionAwaiter.waitForAny showDocAwaiter let newContents = File.ReadAllText(fullPath) diff --git a/src/language-server/Contextive.LanguageServer.Tests/DefinitionsTests.fs b/src/language-server/Contextive.LanguageServer.Tests/DefinitionsTests.fs index 2bd87932..b75a63ce 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/DefinitionsTests.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/DefinitionsTests.fs @@ -137,7 +137,7 @@ let tests = let! definitions = getDefinitionsWithErrorHandler onErrorLoading configGetter workspaceFolder let! termsWhenInvalid = Definitions.find definitions "" id - let! errorMessage = ConditionAwaiter.waitForAny errorMessageAwaiter 500 + let! errorMessage = ConditionAwaiter.waitForAny errorMessageAwaiter test <@ errorMessage.Value = expectedErrorMessage @> test <@ Seq.length termsWhenInvalid = 0 @> diff --git a/src/language-server/Contextive.LanguageServer.Tests/Helpers/ConditionAwaiter.fs b/src/language-server/Contextive.LanguageServer.Tests/Helpers/ConditionAwaiter.fs index 43c4da43..8a8ca601 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/Helpers/ConditionAwaiter.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/Helpers/ConditionAwaiter.fs @@ -17,6 +17,8 @@ type Message<'T> = | WaitFor of WaitForCondition<'T> | Clear +[] +let private DEFAULT_TIMEOUT_MS = 5000 let create<'T> () = let awaiter = @@ -55,16 +57,29 @@ let create<'T> () = let received (awaiter: MailboxProcessor>) msg = awaiter.Post(Received(msg)) -let waitFor (awaiter: MailboxProcessor>) condition timeout = +/// Waits for a message that passes the condition to become true up to the nominated timeout. +let waitForTimeout timeoutMs (awaiter: MailboxProcessor>) condition = awaiter.PostAndTryAsyncReply( (fun rc -> WaitFor( { ReplyChannel = rc Condition = condition } )), - timeout + timeoutMs ) -let waitForAny (awaiter: MailboxProcessor>) timeout = waitFor awaiter (fun _ -> true) timeout +/// Waits for a message that passes the condition to become true up to the default timeout of 5s. +/// If you want a custom timeout, use waitForTimeout. +let waitFor (awaiter: MailboxProcessor>) = + waitForTimeout DEFAULT_TIMEOUT_MS awaiter + +/// Waits for any message up to the nominated timeout. +let waitForAnyTimeout timeoutMs (awaiter: MailboxProcessor>) = + waitForTimeout timeoutMs awaiter (fun _ -> true) + +/// Waits for any message up to the default timeout of 5s. +/// If you want a custom timeout, use waitForAnyTimeout. +let waitForAny awaiter = + waitForAnyTimeout DEFAULT_TIMEOUT_MS awaiter let clear (awaiter: MailboxProcessor>) timeout = awaiter.Post(Clear) diff --git a/src/language-server/Contextive.LanguageServer.Tests/Helpers/ServerLog.fs b/src/language-server/Contextive.LanguageServer.Tests/Helpers/ServerLog.fs index d63f54bf..c1336ef0 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/Helpers/ServerLog.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/Helpers/ServerLog.fs @@ -15,7 +15,7 @@ let waitForLogMessage logAwaiter (logMessage: string) = async { let logCondition = fun (m: string) -> m.Contains(logMessage) - return! ConditionAwaiter.waitFor logAwaiter logCondition 25000 + return! ConditionAwaiter.waitForTimeout 25000 logAwaiter logCondition } let optionsBuilder logAwaiter (b: LanguageClientOptions) = diff --git a/src/language-server/Contextive.LanguageServer.Tests/Helpers/TestClient.fs b/src/language-server/Contextive.LanguageServer.Tests/Helpers/TestClient.fs index 21583366..b39da3a5 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/Helpers/TestClient.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/Helpers/TestClient.fs @@ -17,13 +17,18 @@ type InitializationOptions = type private TestClient() = inherit LanguageServerTestBase(JsonRpcTestOptions()) - override _.SetupServer() = + member _.AddToDisposable languageServer = base.Disposable.Add(languageServer) + + override this.SetupServer() = let clientPipe = Pipe() let serverPipe = Pipe() - setupAndStartLanguageServer (serverPipe.Reader.AsStream()) (clientPipe.Writer.AsStream()) - |> Async.Ignore - |> Async.Start + async { + let! server = setupAndStartLanguageServer (serverPipe.Reader.AsStream()) (clientPipe.Writer.AsStream()) + // this.AddToDisposable server + () + } + |> Async.StartImmediate (clientPipe.Reader.AsStream(), serverPipe.Writer.AsStream()) diff --git a/src/language-server/Contextive.LanguageServer.Tests/InitializationTests.fs b/src/language-server/Contextive.LanguageServer.Tests/InitializationTests.fs index e8c0acf7..0e688d3c 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/InitializationTests.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/InitializationTests.fs @@ -31,6 +31,7 @@ let tests = ConfigurationSection.contextivePathBuilder pathValue ] let! (client, reply) = TestClient(config) |> initAndWaitForReply + use client = client test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @> @@ -43,6 +44,7 @@ let tests = let config = [ ConfigurationSection.contextivePathBuilder $"/tmp/{pathValue}" ] let! (client, reply) = TestClient(config) |> initAndWaitForReply + use client = client test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @> @@ -54,6 +56,7 @@ let tests = let config = [ ConfigurationSection.contextivePathBuilder pathValue ] let! (client, reply) = TestClientWithCustomInitWait(config, Some pathValue) |> initAndWaitForReply + use client = client test <@ client.ClientSettings.Capabilities.Workspace.Configuration.IsSupported @> @@ -72,6 +75,7 @@ let tests = let defaultPath = ".contextive/definitions.yml" let! (client, reply) = TestClient(config) |> initAndWaitForReply + use client = client test <@ (defaultArg reply "").Contains(defaultPath) @> } diff --git a/src/language-server/Contextive.LanguageServer.Tests/Program.fs b/src/language-server/Contextive.LanguageServer.Tests/Program.fs index 4fb06c01..4b4ab80e 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/Program.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/Program.fs @@ -6,4 +6,5 @@ open Contextive.LanguageServer.Program [] let main argv = setupLogging + VerifyTests.VerifierSettings.DisableRequireUniquePrefix() runTestsInAssemblyWithCLIArgs [] argv diff --git a/src/language-server/Contextive.LanguageServer.Tests/WatchedFilesTests.fs b/src/language-server/Contextive.LanguageServer.Tests/WatchedFilesTests.fs index 9506b98f..85060b8b 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/WatchedFilesTests.fs +++ b/src/language-server/Contextive.LanguageServer.Tests/WatchedFilesTests.fs @@ -39,9 +39,9 @@ let tests = ConfigurationSection.contextivePathBuilder $"one.yml" WatchedFiles.optionsBuilder registrationAwaiter ] - let! client = TestClient(config) |> init + use! client = TestClient(config) |> init - let! registrationMsg = ConditionAwaiter.waitForAny registrationAwaiter 500 + let! registrationMsg = ConditionAwaiter.waitForAny registrationAwaiter test <@ client.ClientSettings.Capabilities.Workspace.DidChangeWatchedFiles.IsSupported @> @@ -65,9 +65,9 @@ let tests = ConfigurationSection.contextivePathLoaderBuilder pathLoader WatchedFiles.optionsBuilder registrationAwaiter ] - let! client = TestClient(config) |> init + use! client = TestClient(config) |> init - let! initialRegistrationMsg = ConditionAwaiter.waitForAny registrationAwaiter 500 + let! initialRegistrationMsg = ConditionAwaiter.waitForAny registrationAwaiter ConditionAwaiter.clear registrationAwaiter 500 @@ -81,7 +81,6 @@ let tests = match m with | Registered(_) -> true | _ -> false) - 500 let! unregistrationMsg = ConditionAwaiter.waitFor @@ -90,7 +89,6 @@ let tests = match m with | Unregistered(_) -> true | _ -> false) - 500 match secondRegistrationMsg with | Some(Registered(_, opts)) -> test <@ opts.Watchers |> watcherIsForFile newDefinitionsFile @> @@ -115,7 +113,7 @@ let tests = [ Workspace.optionsBuilder relativePath ConfigurationSection.contextivePathBuilder definitionsFile ] - let! client = TestClient(config) |> init + use! client = TestClient(config) |> init let definitionsFileUri = Path.Combine(Directory.GetCurrentDirectory(), relativePath, definitionsFile) @@ -153,7 +151,7 @@ let tests = [ Workspace.optionsBuilder relativePath ConfigurationSection.contextivePathBuilder definitionsFile ] - let! client = TestClient(config) |> init + use! client = TestClient(config) |> init let definitionsFileUri = Path.Combine(Directory.GetCurrentDirectory(), relativePath, definitionsFile) diff --git a/src/language-server/Contextive.LanguageServer.Tests/fixtures/completion_tests/three.yml b/src/language-server/Contextive.LanguageServer.Tests/fixtures/completion_tests/three.yml index 95c3fc7a..95e8ef1d 100644 --- a/src/language-server/Contextive.LanguageServer.Tests/fixtures/completion_tests/three.yml +++ b/src/language-server/Contextive.LanguageServer.Tests/fixtures/completion_tests/three.yml @@ -5,4 +5,45 @@ contexts: - name: CombinedWord - name: octopus - name: combined - - name: word \ No newline at end of file + - name: word + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm + - name: newterm \ No newline at end of file