From 0d3466659469d8dc55ee9279ec93162e6613f917 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Nov 2025 14:01:28 +0000 Subject: [PATCH 1/5] Initial plan From 3dbb44920d00bcbe79cb635613436e7d22d1172a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Nov 2025 14:15:13 +0000 Subject: [PATCH 2/5] Unify VS install discovery - add helper and update all files Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../VSInstallDiscovery.fs | 8 +++ .../Helpers/AssemblyResolver.fs | 25 ++------- vsintegration/tests/Salsa/VsMocks.fs | 12 +---- .../tests/UnitTests/AssemblyResolver.fs | 51 ++++++++----------- 4 files changed, 35 insertions(+), 61 deletions(-) diff --git a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs index e491ea329bd..e9311b1d1fe 100644 --- a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs +++ b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs @@ -126,3 +126,11 @@ module VSInstallDiscovery = | NotFound reason -> logAction $"Visual Studio installation not found: {reason}" None + + /// Gets the VS installation directory or fails with a detailed error message. + /// This is the recommended method for test scenarios that require VS to be installed. + let getVSInstallDirOrFail () : string = + match tryFindVSInstallation () with + | Found (path, _) -> path + | NotFound reason -> + failwith $"Visual Studio installation not found: {reason}. Ensure VS is installed or environment variables (VSAPPIDDIR, VS*COMNTOOLS) are set." diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs index 78a57793517..112e6a24e7c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs @@ -10,24 +10,7 @@ module AssemblyResolver = open System.Globalization open FSharp.Test.VSInstallDiscovery - let vsInstallDir = - // Use centralized VS installation discovery with graceful fallback - match tryGetVSInstallDir () with - | Some dir -> dir - | None -> - // Fallback to legacy behavior for backward compatibility - let vsvar = - let var = Environment.GetEnvironmentVariable("VS170COMNTOOLS") - - if String.IsNullOrEmpty var then - Environment.GetEnvironmentVariable("VSAPPIDDIR") - else - var - - if String.IsNullOrEmpty vsvar then - failwith "VS170COMNTOOLS and VSAPPIDDIR environment variables not found." - - Path.Combine(vsvar, "..") + let vsInstallDir = getVSInstallDirOrFail () let probingPaths = [| @@ -40,9 +23,9 @@ module AssemblyResolver = |] let addResolver () = - AppDomain.CurrentDomain.add_AssemblyResolve (fun h args -> + AppDomain.CurrentDomain.add_AssemblyResolve (fun _ args -> let found () = - (probingPaths) + probingPaths |> Seq.tryPick (fun p -> try let name = AssemblyName(args.Name) @@ -52,7 +35,7 @@ module AssemblyResolver = name.CodeBase <- codebase name.CultureInfo <- Unchecked.defaultof name.Version <- Unchecked.defaultof - Some(name) + Some name else None with _ -> diff --git a/vsintegration/tests/Salsa/VsMocks.fs b/vsintegration/tests/Salsa/VsMocks.fs index 5189b7faec5..54fe8b906de 100644 --- a/vsintegration/tests/Salsa/VsMocks.fs +++ b/vsintegration/tests/Salsa/VsMocks.fs @@ -1642,6 +1642,7 @@ module internal VsActual = open System.ComponentModel.Composition.Primitives open Microsoft.VisualStudio.Text open Microsoft.VisualStudio.Threading + open FSharp.Test.VSInstallDiscovery type TestExportJoinableTaskContext () = @@ -1650,16 +1651,7 @@ module internal VsActual = [)>] member public _.JoinableTaskContext : JoinableTaskContext = jtc - let vsInstallDir = - // use the environment variable to find the VS installdir - let vsvar = - let var = Environment.GetEnvironmentVariable("VS170COMNTOOLS") - if String.IsNullOrEmpty var then - Environment.GetEnvironmentVariable("VSAPPIDDIR") - else - var - if String.IsNullOrEmpty vsvar then failwith "VS170COMNTOOLS and VSAPPIDDIR environment variables not found." - Path.Combine(vsvar, "..") + let vsInstallDir = getVSInstallDirOrFail () let CreateEditorCatalog() = let thisAssembly = Assembly.GetExecutingAssembly().Location diff --git a/vsintegration/tests/UnitTests/AssemblyResolver.fs b/vsintegration/tests/UnitTests/AssemblyResolver.fs index 38b5ee45290..c644cacefd7 100644 --- a/vsintegration/tests/UnitTests/AssemblyResolver.fs +++ b/vsintegration/tests/UnitTests/AssemblyResolver.fs @@ -8,34 +8,23 @@ module AssemblyResolver = open System.Globalization open FSharp.Test.VSInstallDiscovery - let vsInstallDir = - // Use centralized VS installation discovery with graceful fallback - match tryGetVSInstallDir () with - | Some dir -> dir - | None -> - // Fallback to legacy behavior for backward compatibility - let vsvar = - let var = Environment.GetEnvironmentVariable("VS170COMNTOOLS") - if String.IsNullOrEmpty var then - Environment.GetEnvironmentVariable("VSAPPIDDIR") - else - var - if String.IsNullOrEmpty vsvar then failwith "VS170COMNTOOLS and VSAPPIDDIR environment variables not found." - Path.Combine(vsvar, "..") + let vsInstallDir = getVSInstallDirOrFail () - let probingPaths = [| - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\Editor") - Path.Combine(vsInstallDir, @"IDE\PublicAssemblies") - Path.Combine(vsInstallDir, @"IDE\PrivateAssemblies") - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\ManagedLanguages\VBCSharp\LanguageServices") - Path.Combine(vsInstallDir, @"IDE\Extensions\Microsoft\CodeSense\Framework") - Path.Combine(vsInstallDir, @"IDE") - |] + let probingPaths = + [| + Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\Editor") + Path.Combine(vsInstallDir, @"IDE\PublicAssemblies") + Path.Combine(vsInstallDir, @"IDE\PrivateAssemblies") + Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\ManagedLanguages\VBCSharp\LanguageServices") + Path.Combine(vsInstallDir, @"IDE\Extensions\Microsoft\CodeSense\Framework") + Path.Combine(vsInstallDir, @"IDE") + |] let addResolver () = - AppDomain.CurrentDomain.add_AssemblyResolve(fun h args -> + AppDomain.CurrentDomain.add_AssemblyResolve(fun _ args -> let found () = - (probingPaths ) |> Seq.tryPick(fun p -> + probingPaths + |> Seq.tryPick (fun p -> try let name = AssemblyName(args.Name) let codebase = Path.GetFullPath(Path.Combine(p, name.Name) + ".dll") @@ -43,10 +32,12 @@ module AssemblyResolver = name.CodeBase <- codebase name.CultureInfo <- Unchecked.defaultof name.Version <- Unchecked.defaultof - Some (name) - else None - with | _ -> None - ) - match found() with + Some name + else + None + with _ -> + None) + + match found () with | None -> Unchecked.defaultof - | Some name -> Assembly.Load(name) ) + | Some name -> Assembly.Load(name)) From f4117d4c17f46ee2ee7ae2a2581bb4c97955490e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Nov 2025 18:03:48 +0000 Subject: [PATCH 3/5] Consolidate AssemblyResolver logic and fix Salsa project reference - Move AssemblyResolver logic to VSInstallDiscovery.fs as a new module - Update both AssemblyResolver.fs files to delegate to centralized implementation - Add VSInstallDiscovery.fs as linked file to Salsa project (fixes missing reference) - Reduces code duplication further (~70 lines total removed) Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../VSInstallDiscovery.fs | 44 +++++++++++++++++++ .../Helpers/AssemblyResolver.fs | 44 ++----------------- .../tests/Salsa/VisualFSharp.Salsa.fsproj | 3 ++ .../tests/UnitTests/AssemblyResolver.fs | 43 ++---------------- 4 files changed, 55 insertions(+), 79 deletions(-) diff --git a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs index e9311b1d1fe..7c4d9551a98 100644 --- a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs +++ b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs @@ -134,3 +134,47 @@ module VSInstallDiscovery = | Found (path, _) -> path | NotFound reason -> failwith $"Visual Studio installation not found: {reason}. Ensure VS is installed or environment variables (VSAPPIDDIR, VS*COMNTOOLS) are set." + +/// Assembly resolver for Visual Studio test infrastructure. +/// Provides centralized assembly resolution for VS integration tests. +module AssemblyResolver = + open System + open System.IO + open System.Reflection + open System.Globalization + + /// Adds an assembly resolver that probes Visual Studio installation directories. + /// This should be called early in test initialization to ensure VS assemblies can be loaded. + let addResolver () = + let vsInstallDir = VSInstallDiscovery.getVSInstallDirOrFail () + + let probingPaths = + [| + Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\Editor") + Path.Combine(vsInstallDir, @"IDE\PublicAssemblies") + Path.Combine(vsInstallDir, @"IDE\PrivateAssemblies") + Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\ManagedLanguages\VBCSharp\LanguageServices") + Path.Combine(vsInstallDir, @"IDE\Extensions\Microsoft\CodeSense\Framework") + Path.Combine(vsInstallDir, @"IDE") + |] + + AppDomain.CurrentDomain.add_AssemblyResolve(fun _ args -> + let found () = + probingPaths + |> Seq.tryPick (fun p -> + try + let name = AssemblyName(args.Name) + let codebase = Path.GetFullPath(Path.Combine(p, name.Name) + ".dll") + if File.Exists(codebase) then + name.CodeBase <- codebase + name.CultureInfo <- Unchecked.defaultof + name.Version <- Unchecked.defaultof + Some name + else + None + with _ -> + None) + + match found () with + | None -> Unchecked.defaultof + | Some name -> Assembly.Load(name)) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs index 112e6a24e7c..6e7d14e6087 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs @@ -2,45 +2,9 @@ namespace FSharp.Editor.Tests.Helpers -open System -open System.IO -open System.Reflection - module AssemblyResolver = - open System.Globalization - open FSharp.Test.VSInstallDiscovery - - let vsInstallDir = getVSInstallDirOrFail () - - let probingPaths = - [| - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\Editor") - Path.Combine(vsInstallDir, @"IDE\PublicAssemblies") - Path.Combine(vsInstallDir, @"IDE\PrivateAssemblies") - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\ManagedLanguages\VBCSharp\LanguageServices") - Path.Combine(vsInstallDir, @"IDE\Extensions\Microsoft\CodeSense\Framework") - Path.Combine(vsInstallDir, @"IDE") - |] - - let addResolver () = - AppDomain.CurrentDomain.add_AssemblyResolve (fun _ args -> - let found () = - probingPaths - |> Seq.tryPick (fun p -> - try - let name = AssemblyName(args.Name) - let codebase = Path.GetFullPath(Path.Combine(p, name.Name) + ".dll") - - if File.Exists(codebase) then - name.CodeBase <- codebase - name.CultureInfo <- Unchecked.defaultof - name.Version <- Unchecked.defaultof - Some name - else - None - with _ -> - None) + open FSharp.Test.AssemblyResolver - match found () with - | None -> Unchecked.defaultof - | Some name -> Assembly.Load(name)) + /// Adds an assembly resolver that probes Visual Studio installation directories. + /// This is a compatibility shim that delegates to the centralized implementation. + let addResolver = addResolver diff --git a/vsintegration/tests/Salsa/VisualFSharp.Salsa.fsproj b/vsintegration/tests/Salsa/VisualFSharp.Salsa.fsproj index f7410214430..d41b116c21e 100644 --- a/vsintegration/tests/Salsa/VisualFSharp.Salsa.fsproj +++ b/vsintegration/tests/Salsa/VisualFSharp.Salsa.fsproj @@ -24,6 +24,9 @@ UnitTests.TestLib.Utils.fs + + VSInstallDiscovery.fs + diff --git a/vsintegration/tests/UnitTests/AssemblyResolver.fs b/vsintegration/tests/UnitTests/AssemblyResolver.fs index c644cacefd7..620f4ba34fc 100644 --- a/vsintegration/tests/UnitTests/AssemblyResolver.fs +++ b/vsintegration/tests/UnitTests/AssemblyResolver.fs @@ -1,43 +1,8 @@ namespace Microsoft.VisualStudio.FSharp -open System -open System.IO -open System.Reflection - module AssemblyResolver = - open System.Globalization - open FSharp.Test.VSInstallDiscovery - - let vsInstallDir = getVSInstallDirOrFail () - - let probingPaths = - [| - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\Editor") - Path.Combine(vsInstallDir, @"IDE\PublicAssemblies") - Path.Combine(vsInstallDir, @"IDE\PrivateAssemblies") - Path.Combine(vsInstallDir, @"IDE\CommonExtensions\Microsoft\ManagedLanguages\VBCSharp\LanguageServices") - Path.Combine(vsInstallDir, @"IDE\Extensions\Microsoft\CodeSense\Framework") - Path.Combine(vsInstallDir, @"IDE") - |] - - let addResolver () = - AppDomain.CurrentDomain.add_AssemblyResolve(fun _ args -> - let found () = - probingPaths - |> Seq.tryPick (fun p -> - try - let name = AssemblyName(args.Name) - let codebase = Path.GetFullPath(Path.Combine(p, name.Name) + ".dll") - if File.Exists(codebase) then - name.CodeBase <- codebase - name.CultureInfo <- Unchecked.defaultof - name.Version <- Unchecked.defaultof - Some name - else - None - with _ -> - None) + open FSharp.Test.AssemblyResolver - match found () with - | None -> Unchecked.defaultof - | Some name -> Assembly.Load(name)) + /// Adds an assembly resolver that probes Visual Studio installation directories. + /// This is a compatibility shim that delegates to the centralized implementation. + let addResolver = addResolver From 19196b7fddf6bef7b930e83e252722a73ff6f3de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 13 Nov 2025 18:18:33 +0000 Subject: [PATCH 4/5] Suppress warning 44 for deprecated CodeBase property The AssemblyName.CodeBase property is deprecated but still needed for assembly resolution in VS integration tests. Suppress warning 44 in FSharp.Test.Utilities project. Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj index 7bc4f6e058f..d899b551e30 100644 --- a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj +++ b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj @@ -102,7 +102,8 @@ - $(NoWarn);NU1510 + $(NoWarn);NU1510;44 + From 86177e3aed6623a6e3a5c0672c9f1ea039692585 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 27 Nov 2025 13:35:09 +0000 Subject: [PATCH 5/5] Rename AssemblyResolver to VSAssemblyResolver to avoid conflict Fixes FS0248 error: Two modules named 'FSharp.Test.AssemblyResolver' were occurring because CompilerAssert.fs already has an AssemblyResolver module. Renamed the new module to VSAssemblyResolver to avoid conflict. Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- tests/FSharp.Test.Utilities/VSInstallDiscovery.fs | 2 +- .../tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs | 2 +- vsintegration/tests/UnitTests/AssemblyResolver.fs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs index 7c4d9551a98..e59eec8eed2 100644 --- a/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs +++ b/tests/FSharp.Test.Utilities/VSInstallDiscovery.fs @@ -137,7 +137,7 @@ module VSInstallDiscovery = /// Assembly resolver for Visual Studio test infrastructure. /// Provides centralized assembly resolution for VS integration tests. -module AssemblyResolver = +module VSAssemblyResolver = open System open System.IO open System.Reflection diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs index 6e7d14e6087..ef8c01ca908 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs @@ -3,7 +3,7 @@ namespace FSharp.Editor.Tests.Helpers module AssemblyResolver = - open FSharp.Test.AssemblyResolver + open FSharp.Test.VSAssemblyResolver /// Adds an assembly resolver that probes Visual Studio installation directories. /// This is a compatibility shim that delegates to the centralized implementation. diff --git a/vsintegration/tests/UnitTests/AssemblyResolver.fs b/vsintegration/tests/UnitTests/AssemblyResolver.fs index 620f4ba34fc..cf36b723e40 100644 --- a/vsintegration/tests/UnitTests/AssemblyResolver.fs +++ b/vsintegration/tests/UnitTests/AssemblyResolver.fs @@ -1,7 +1,7 @@ namespace Microsoft.VisualStudio.FSharp module AssemblyResolver = - open FSharp.Test.AssemblyResolver + open FSharp.Test.VSAssemblyResolver /// Adds an assembly resolver that probes Visual Studio installation directories. /// This is a compatibility shim that delegates to the centralized implementation.