Upd4 (#138)

dsyme · web-flow · commit 5bc6881ae488 · 2022-03-19T22:49:48.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.57.0] - 2022-03-20
+
+### Changed
+
+- ProjectController.LoadProject now returns an Async bool to indicate eventual completion
+- Fix heisen test https://github.com/ionide/proj-info/issues/136
+- Multiple agents being created by ProjectSystem
+
 ## [0.56.0] - 2022-03-19
 
 ### Changed
diff --git a/src/Ionide.ProjInfo.ProjectSystem/Project.fs b/src/Ionide.ProjInfo.ProjectSystem/Project.fs
@@ -107,9 +107,9 @@ type internal ProjectPersistentCache(projectFile: string) =
 
             loop ()
 
-    member __.SaveCache(lwt, resp) = agent.Post(Save(lwt, resp))
+    member _.SaveCache(lwt, resp) = agent.Post(Save(lwt, resp))
 
-    member __.LoadCache(lwt) =
+    member _.LoadCache(lwt) =
         agent.PostAndReply(fun ch -> Load(lwt, ch))
 
 type private ProjectMessage =
@@ -204,14 +204,14 @@ type internal Project(projectFile, onChange: string -> unit) =
 
     do propsfsw.EnableRaisingEvents <- true
 
-    member __.Response
+    member _.Response
         with get () = agent.PostAndReply GetResponse
         and set r = agent.Post(SetResponse r)
 
-    member __.FileName = fullPath
+    member _.FileName = fullPath
 
     interface IDisposable with
-        member __.Dispose() =
+        member _.Dispose() =
             propsfsw.Dispose()
             afsw.Dispose()
             fsw.Dispose()
diff --git a/src/Ionide.ProjInfo.ProjectSystem/ProjectSystem.fs b/src/Ionide.ProjInfo.ProjectSystem/ProjectSystem.fs
@@ -43,7 +43,6 @@ type ProjectController(toolsPath: ToolsPath, workspaceLoaderFactory: ToolsPath -
     let workspaceReady = Event<unit>()
     let notify = Event<ProjectResponse>()
 
-
     let deduplicateBy keySelector (obs: IObservable<'a>) =
         obs
         |> Observable.synchronize // deals with concurrency issues
@@ -61,37 +60,35 @@ type ProjectController(toolsPath: ToolsPath, workspaceLoaderFactory: ToolsPath -
             projs |> List.iter (fun (fileName, _) -> fileName |> ProjectResponse.ProjectChanged |> notify.Trigger)
 
             for (key, group) in projectGroups do
-                x.LoadWorkspace(group, key)
+                x.LoadWorkspace(group, key) |> ignore
 
         projectsChanged |> deduplicateBy fst |> Observable.subscribe loadProjects
 
 
     let updateState (response: ProjectCrackerCache) =
         let normalizeOptions (opts: FSharpProjectOptions) =
             { opts with
-                  SourceFiles =
+                SourceFiles =
                     opts.SourceFiles
                     |> Array.filter (FscArguments.isCompileFile)
                     |> Array.map (Path.GetFullPath)
                     |> Array.map (fun p -> (p.Chars 0).ToString().ToLower() + p.Substring(1))
-                  OtherOptions =
-                      opts.OtherOptions
-                      |> Array.map
-                          (fun n ->
-                              if FscArguments.isCompileFile (n) then
-                                  Path.GetFullPath n
-                              else
-                                  n) }
+                OtherOptions =
+                    opts.OtherOptions
+                    |> Array.map (fun n ->
+                        if FscArguments.isCompileFile (n) then
+                            Path.GetFullPath n
+                        else
+                            n) }
 
         for file in
             response.Items
-            |> List.choose
-                (function
+            |> List.choose (function
                 | ProjectViewerItem.Compile (p, _) -> Some p) do
             fileCheckOptions.[file] <- normalizeOptions response.Options
 
 
-    member private x.loadProjects (files: string list) (binaryLogs: BinaryLogGeneration) =
+    let loadProjects (files: string list) (binaryLogs: BinaryLogGeneration) =
         async {
             let onChange fn = projectsChanged.OnNext(fn, binaryLogs)
 
@@ -117,8 +114,7 @@ type ProjectController(toolsPath: ToolsPath, workspaceLoaderFactory: ToolsPath -
 
                     let responseFiles =
                         response.Items
-                        |> List.choose
-                            (function
+                        |> List.choose (function
                             | ProjectViewerItem.Compile (p, _) -> Some p)
 
                     let projInfo: ProjectResult =
@@ -134,8 +130,7 @@ type ProjectController(toolsPath: ToolsPath, workspaceLoaderFactory: ToolsPath -
                 | ProjectSystemState.LoadedOther (extraInfo, projectFiles, fromDpiCache) ->
                     let responseFiles =
                         projectFiles
-                        |> List.choose
-                            (function
+                        |> List.choose (function
                             | ProjectViewerItem.Compile (p, _) -> Some p)
 
                     let projInfo: ProjectResult =
@@ -177,75 +172,77 @@ type ProjectController(toolsPath: ToolsPath, workspaceLoaderFactory: ToolsPath -
             return true
         }
 
-    member private x.LoaderLoop =
-        MailboxProcessor.Start
-            (fun agent -> //If couldn't recive new event in 50 ms then just load previous one
-                let rec loop (previousStatus: (string list * BinaryLogGeneration) option) =
-                    async {
-                        match previousStatus with
-
-                        | Some (fn, gb) ->
-                            match! agent.TryReceive(50) with
-                            | None -> //If couldn't recive new event in 50 ms then just load previous one
-                                let! _ = x.loadProjects fn gb
-                                return! loop None
-                            | Some (fn2, gb2) when fn2 = fn -> //If recived same load request then wait again (in practice shouldn't happen more than 2 times)
-                                return! loop previousStatus
-                            | Some (fn2, gb2) -> //If recived some other project load previous one, and then wait with the new one
-                                let! _ = x.loadProjects fn gb
-                                return! loop (Some(fn2, gb2))
-                        | None ->
-                            let! (fn, gb) = agent.Receive()
-                            return! loop (Some(fn, gb))
-                    }
-
-                loop None)
+    let loaderLoop =
+        MailboxProcessor.Start (fun agent -> //If couldn't recive new event in 50 ms then just load previous one
+            let rec loop (previousStatus: (AsyncReplyChannel<bool> * string list * BinaryLogGeneration) option) =
+                async {
+                    match previousStatus with
+
+                    | Some (chan, fn, gb) ->
+                        match! agent.TryReceive(50) with
+                        | None -> //If couldn't recive new event in 50 ms then just load previous one
+                            let! res = loadProjects fn gb
+                            chan.Reply res
+                            return! loop None
+                        | Some (chan2, fn2, gb2) when fn2 = fn -> //If recived same load request then wait again (in practice shouldn't happen more than 2 times)
+                            return! loop previousStatus
+                        | Some (chan2, fn2, gb2) -> //If recived some other project load previous one, and then wait with the new one
+                            let! res = loadProjects fn gb
+                            chan.Reply res
+                            return! loop (Some(chan2, fn2, gb2))
+                    | None ->
+                        let! (chan, fn, gb) = agent.Receive()
+                        return! loop (Some(chan, fn, gb))
+                }
+
+            loop None)
 
     ///Event notifies that whole workspace has been loaded
-    member __.WorkspaceReady = workspaceReady.Publish
+    member _.WorkspaceReady = workspaceReady.Publish
 
     ///Event notifies about any loading events
-    member __.Notifications = notify.Publish
+    member _.Notifications = notify.Publish
 
-    member __.IsWorkspaceReady = isWorkspaceReady
+    member _.IsWorkspaceReady = isWorkspaceReady
 
     ///Try to get instance of `FSharpProjectOptions` for given `.fs` file
-    member __.GetProjectOptions(file: string) : FSharpProjectOptions option =
+    member _.GetProjectOptions(file: string) : FSharpProjectOptions option =
         let file = Utils.normalizePath file
         fileCheckOptions.TryFind file
 
-    member __.SetProjectOptions(file: string, opts: FSharpProjectOptions) =
+    member _.SetProjectOptions(file: string, opts: FSharpProjectOptions) =
         let file = Utils.normalizePath file
         fileCheckOptions.AddOrUpdate(file, (fun _ -> opts), (fun _ _ -> opts)) |> ignore
 
-    member __.RemoveProjectOptions(file) =
+    member _.RemoveProjectOptions(file) =
         let file = Utils.normalizePath file
         fileCheckOptions.TryRemove file |> ignore
 
     ///Try to get instance of `FSharpProjectOptions` for given `.fsproj` file
-    member __.GetProjectOptionsForFsproj(fsprojPath: string) : FSharpProjectOptions option =
+    member _.GetProjectOptionsForFsproj(fsprojPath: string) : FSharpProjectOptions option =
         fileCheckOptions.Values |> Seq.tryFind (fun n -> n.ProjectFileName = fsprojPath)
 
     ///Returns a sequance of all known path-to-`.fs` * `FSharpProjectOptions` pairs
-    member __.ProjectOptions = fileCheckOptions |> Seq.map (|KeyValue|)
+    member _.ProjectOptions = fileCheckOptions |> Seq.map (|KeyValue|)
 
     ///Loads a single project file
     member x.LoadProject(projectFileName: string, binaryLogs: BinaryLogGeneration) =
-        x.LoaderLoop.Post([ projectFileName ], binaryLogs)
+        loaderLoop.PostAndAsyncReply((fun chan -> chan, [ projectFileName ], binaryLogs))
 
     ///Loads a single project file
     member x.LoadProject(projectFileName: string) =
         x.LoadProject(projectFileName, BinaryLogGeneration.Off)
 
     ///Loads a set of project files
-    member x.LoadWorkspace(files: string list, binaryLogs: BinaryLogGeneration) = x.LoaderLoop.Post(files, binaryLogs)
+    member x.LoadWorkspace(files: string list, binaryLogs: BinaryLogGeneration) =
+        loaderLoop.PostAndAsyncReply((fun chan -> chan, files, binaryLogs))
 
     ///Loads a set of project files
     member x.LoadWorkspace(files: string list) =
         x.LoadWorkspace(files, BinaryLogGeneration.Off)
 
     ///Finds a list of potential workspaces (solution files/lists of projects) in given dir
-    member __.PeekWorkspace(dir: string, deep: int, excludedDirs: string list) =
+    member _.PeekWorkspace(dir: string, deep: int, excludedDirs: string list) =
         WorkspacePeek.peek dir deep excludedDirs
 
     interface IDisposable with
diff --git a/test/Ionide.ProjInfo.Tests/FileUtils.fs b/test/Ionide.ProjInfo.Tests/FileUtils.fs
@@ -101,19 +101,19 @@ let touch (logger: Logger) path =
 type FileUtils(logger: Logger) =
     let mutable currentDirectory = Environment.CurrentDirectory
 
-    member __.cd dir =
+    member _.cd dir =
         logger.debug (eventX "cd '{directory}'" >> setField "directory" dir)
         currentDirectory <- dir
 
-    member __.rm_rf = rm_rf logger
-    member __.mkdir_p = mkdir_p logger
-    member __.cp = cp logger
-    member __.cp_r = cp_r logger
-    member __.shellExecRun = shellExecRun logger currentDirectory
-    member __.shellExecRunNET = shellExecRunNET logger currentDirectory
-    member __.createFile = createFile logger
-    member __.unzip = unzip logger
-    member __.readFile = readFile logger
-    member __.touch = touch logger
+    member _.rm_rf = rm_rf logger
+    member _.mkdir_p = mkdir_p logger
+    member _.cp = cp logger
+    member _.cp_r = cp_r logger
+    member _.shellExecRun = shellExecRun logger currentDirectory
+    member _.shellExecRunNET = shellExecRunNET logger currentDirectory
+    member _.createFile = createFile logger
+    member _.unzip = unzip logger
+    member _.readFile = readFile logger
+    member _.touch = touch logger
 
 let writeLines (lines: string list) (stream: StreamWriter) = lines |> List.iter stream.WriteLine
diff --git a/test/Ionide.ProjInfo.Tests/Tests.fs b/test/Ionide.ProjInfo.Tests/Tests.fs
@@ -73,11 +73,8 @@ let createFCS () =
     checker
 
 let sleepABit () =
-    // we wait a bit longer on macos in CI due to apparent slowness
-    if System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(System.Runtime.InteropServices.OSPlatform.OSX) then
-        System.Threading.Thread.Sleep 5000
-    else
-        System.Threading.Thread.Sleep 3000
+    // CI has apparent occasional slowness
+    System.Threading.Thread.Sleep 5000
 
 [<AutoOpen>]
 module ExpectNotification =
@@ -131,7 +128,7 @@ module ExpectNotification =
                 notifications.Add(arg)
                 log arg)
 
-        member __.Notifications = notifications |> List.ofSeq
+        member _.Notifications = notifications |> List.ofSeq
 
     let logNotification (logger: Logger) arg =
         logger.debug (eventX "notified: {notification}'" >> setField "notification" arg)
@@ -634,8 +631,7 @@ let testProjectNotFound toolsPath workspaceLoader (workspaceFactory: ToolsPath -
 
         Expect.equal parsed.Length 0 "no project loaded"
 
-        Expect.equal (watcher.Notifications |> List.item 1) (WorkspaceProjectState.Failed(wrongPath, (GetProjectOptionsErrors.ProjectNotFound(wrongPath)))) "check error type"
-    )
+        Expect.equal (watcher.Notifications |> List.item 1) (WorkspaceProjectState.Failed(wrongPath, (GetProjectOptionsErrors.ProjectNotFound(wrongPath)))) "check error type")
 
 let internalGetProjectOptions =
     fun (r: FSharpReferencedProject) ->
@@ -683,7 +679,8 @@ let testFCSmap toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorksp
         let parsed = loader.LoadProjects [ projPath ] |> Seq.toList
         let mutable pos = Map.empty
 
-        loader.Notifications.Add (function | WorkspaceProjectState.Loaded (po, knownProjects, _) -> pos <- Map.add po.ProjectFileName po pos)
+        loader.Notifications.Add (function
+            | WorkspaceProjectState.Loaded (po, knownProjects, _) -> pos <- Map.add po.ProjectFileName po pos)
 
         let fcsPo = FCS.mapToFSharpProjectOptions parsed.Head parsed
 
@@ -707,7 +704,7 @@ let testFCSmap toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorksp
 
         Expect.isNonEmpty uses "all symbols usages"
 
-        )
+    )
 
 let testFCSmapManyProj toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
     testCase
@@ -733,7 +730,7 @@ let testFCSmapManyProj toolsPath workspaceLoader (workspaceFactory: ToolsPath ->
                 Expect.equal tar dpoPo.TargetPath (sprintf "p2p key is TargetPath, fsc projet options was '%A'" fcsPO)
 
         let testDir = inDir fs "load_sample_fsc"
-        copyDirFromAssets fs  ``sample3 Netsdk projs``.ProjDir testDir
+        copyDirFromAssets fs ``sample3 Netsdk projs``.ProjDir testDir
 
         let projPath = testDir / (``sample3 Netsdk projs``.ProjectFile)
 
@@ -744,7 +741,8 @@ let testFCSmapManyProj toolsPath workspaceLoader (workspaceFactory: ToolsPath ->
         let parsed = loader.LoadProjects [ projPath ] |> Seq.toList
         let mutable pos = Map.empty
 
-        loader.Notifications.Add (function | WorkspaceProjectState.Loaded (po, knownProjects, _) -> pos <- Map.add po.ProjectFileName po pos)
+        loader.Notifications.Add (function
+            | WorkspaceProjectState.Loaded (po, knownProjects, _) -> pos <- Map.add po.ProjectFileName po pos)
 
         let fcsPo = FCS.mapToFSharpProjectOptions parsed.Head parsed
         let hasCSharpRef = fcsPo.OtherOptions |> Seq.exists (fun opt -> opt.StartsWith "-r:" && opt.EndsWith "l1.dll")
@@ -756,7 +754,7 @@ let testFCSmapManyProj toolsPath workspaceLoader (workspaceFactory: ToolsPath ->
         Expect.equal hasFSharpRef true "Should have direct dll reference to F# reference"
         Expect.equal hasFSharpProjectRef true "Should have project reference to F# reference"
 
-        )
+    )
 
 let testSample2WithBinLog toolsPath workspaceLoader (workspaceFactory: ToolsPath -> IWorkspaceLoader) =
     testCase
@@ -870,7 +868,7 @@ module ExpectProjectSystemNotification =
                 notifications.Add(arg)
                 log arg)
 
-        member __.Notifications = notifications |> List.ofSeq
+        member _.Notifications = notifications |> List.ofSeq
 
     let logNotification (logger: Logger) arg =
         logger.debug (eventX "notified: {notification}'" >> setField "notification" arg)
@@ -890,9 +888,8 @@ let testLoadProject toolsPath =
 
         let projResult = ProjectLoader.getProjectInfo projPath [] BinaryLogGeneration.Off []
 
-        match projResult with 
-        | Result.Ok proj -> 
-          Expect.equal proj.ProjectFileName projPath "project file names"
+        match projResult with
+        | Result.Ok proj -> Expect.equal proj.ProjectFileName projPath "project file names"
         | Result.Error err -> failwith $"{err}"
 
     )
@@ -910,9 +907,9 @@ let testProjectSystem toolsPath workspaceLoader workspaceFactory =
 
         use controller = new ProjectSystem.ProjectController(toolsPath, workspaceFactory)
         let watcher = watchNotifications logger controller
-        controller.LoadProject(projPath)
+        let result = controller.LoadProject(projPath) |> Async.RunSynchronously
 
-        sleepABit ()
+        Expect.isTrue result "load succeeds"
 
         let parsed = controller.ProjectOptions |> Seq.toList |> List.map (snd)
         let fcsPo = parsed.Head
@@ -950,10 +947,8 @@ let testProjectSystemOnChange toolsPath workspaceLoader workspaceFactory =
 
         use controller = new ProjectSystem.ProjectController(toolsPath, workspaceFactory)
         let watcher = watchNotifications logger controller
-        controller.LoadProject(projPath)
-
-        sleepABit ()
-
+        let result = controller.LoadProject(projPath) |> Async.RunSynchronously
+        Expect.isTrue result "load succeeds"
 
         [ workspace false
           loading "n1.fsproj"
