test: improve language-server test resilience

chrissimon-au · chrissimon-au · commit ee9aa7546246 · 2023-11-26T22:02:58.000Z
primarily by increasing timeouts, ensuring testclient and testlanguageserver is disposed properly, and adding missing conditionawaiters. Likely still a testclient memory leak - see OmniSharp/csharp-language-server-protocol#1037
diff --git 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
@@ -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
@@ -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
@@ -17,6 +17,8 @@ type Message<'T> =
     | WaitFor of WaitForCondition<'T>
     | Clear
 
+[<Literal>]
+let private DEFAULT_TIMEOUT_MS = 5000
 
 let create<'T> () =
     let awaiter =
@@ -55,16 +57,29 @@ let create<'T> () =
 
 let received (awaiter: MailboxProcessor<Message<'T>>) msg = awaiter.Post(Received(msg))
 
-let waitFor (awaiter: MailboxProcessor<Message<'T>>) condition timeout =
+/// Waits for a message that passes the condition to become true up to the nominated timeout.
+let waitForTimeout timeoutMs (awaiter: MailboxProcessor<Message<'T>>) condition =
     awaiter.PostAndTryAsyncReply(
         (fun rc ->
             WaitFor(
                 { ReplyChannel = rc
                   Condition = condition }
             )),
-        timeout
+        timeoutMs
     )
 
-let waitForAny (awaiter: MailboxProcessor<Message<'T>>) 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<Message<'T>>) =
+    waitForTimeout DEFAULT_TIMEOUT_MS awaiter
+
+/// Waits for any message up to the nominated timeout.
+let waitForAnyTimeout timeoutMs (awaiter: MailboxProcessor<Message<'T>>) =
+    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<Message<'T>>) 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
@@ -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
@@ -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
@@ -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
@@ -6,4 +6,5 @@ open Contextive.LanguageServer.Program
 [<EntryPoint>]
 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
@@ -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
@@ -5,4 +5,45 @@ contexts:
     - name: CombinedWord
     - name: octopus
     - name: combined
-    - name: word
+    - 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

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ let waitForLogMessage logAwaiter (logMessage: string) =`
`15`	`15`	`async {`
`16`	`16`	`let logCondition = fun (m: string) -> m.Contains(logMessage)`
`17`	`17`
`18`		`- return! ConditionAwaiter.waitFor logAwaiter logCondition 25000`
	`18`	`+ return! ConditionAwaiter.waitForTimeout 25000 logAwaiter logCondition`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`let optionsBuilder logAwaiter (b: LanguageClientOptions) =`