Centralize Visual Studio installation discovery and assembly resolution in test infrastructure

[Copilot]

Description

Consolidates VS installation discovery and assembly resolution logic scattered across test files. Four files had duplicate fallback code; VsMocks.fs hardcoded VS170COMNTOOLS instead of using centralized discovery. Two AssemblyResolver.fs files contained identical assembly resolution logic.

Changes

Added to VSInstallDiscovery.fs:

• getVSInstallDirOrFail() - fails with detailed error showing discovery methods attempted
• VSAssemblyResolver module - centralizes assembly resolution logic for VS integration tests (named to avoid conflict with existing AssemblyResolver module in CompilerAssert.fs)

Updated consumer files:

• Removed ~116 lines of duplicate code across files (net reduction of 49 lines)
• Both AssemblyResolver.fs files now delegate to centralized FSharp.Test.VSAssemblyResolver module
• VsMocks.fs uses centralized discovery via linked VSInstallDiscovery.fs file
• Fixed unused parameter warnings (fun h args → fun _ args)

Project structure changes:

• Added VSInstallDiscovery.fs as linked file to Salsa project (avoids NuGet reference warnings)
• Suppressed warning 44 for deprecated AssemblyName.CodeBase property (still needed for assembly resolution)

Impact

• VS version updates (e.g., VS 2026) require changes in one place only
• Assembly resolution logic shared across all VS integration test projects
• Error messages now show all discovery attempts (VSAPPIDDIR, VS*COMNTOOLS, vswhere.exe)
• No circular dependencies - Salsa project uses linked file instead of package reference
• Test infrastructure uses identical discovery and resolution logic across all files

Checklist

• Test cases added
• Performance benchmarks added in case of performance changes
• Release notes entry updated:

  NO_RELEASE_NOTES - Internal test infrastructure refactoring only, no user-facing changes

Original prompt

Unify Visual Studio Installation Discovery Across Test Infrastructure

Overview

Currently, the codebase has Visual Studio installation discovery logic duplicated across multiple test files, with some files still hardcoding specific VS version environment variables (like VS170COMNTOOLS). We have a centralized VSInstallDiscovery.fs module that handles this properly, but not all files are using it correctly.

Goals

1. Remove redundant fallback logic from all consumer files
2. Add a new helper function to centralize error handling
3. Update all files to use the centralized discovery mechanism
4. Ensure consistent formatting and error messages across all files

Current Problems

• Duplicated fallback code: Three files have identical 8-line fallback logic that's redundant
• Hardcoded version: VsMocks.fs still hardcodes VS170COMNTOOLS instead of using centralized discovery
• Inconsistent error messages: Different files fail with different messages
• Poor diagnostics: Error messages don't explain what discovery methods were tried

Files to Modify

1. tests/FSharp.Test.Utilities/VSInstallDiscovery.fs

Add a new helper function at the end of the file (after the existing getVSInstallDirWithLogging function):

    /// 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."

This should be added after line 128 (after the getVSInstallDirWithLogging function).

2. vsintegration/tests/UnitTests/AssemblyResolver.fs

Replace the entire file content with:

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<CultureInfo>
                            name.Version <- Unchecked.defaultof<Version>
                            Some name
                        else
                            None
                    with _ ->
                        None)

            match found () with
            | None -> Unchecked.defaultof<Assembly>
            | Some name -> Assembly.Load(name))

Key changes from original:

• Removed lines 11-24 (redundant fallback logic)
• Changed line 11 from match tryGetVSInstallDir()... to let vsInstallDir = getVSInstallDirOrFail ()
• Changed fun h args -> to fun _ args -> (unused parameter)
• Removed extra spaces in (probingPaths ) → probingPaths
• Consistent formatting

3. vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs

Replace the entire file content with:

// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.

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 () =
        App...

</details>

*This pull request was created as a result of the following prompt from Copilot chat.*
> 
> # Unify Visual Studio Installation Discovery Across Test Infrastructure
> 
> ## Overview
> 
> Currently, the codebase has Visual Studio installation discovery logic duplicated across multiple test files, with some files still hardcoding specific VS version environment variables (like `VS170COMNTOOLS`). We have a centralized `VSInstallDiscovery.fs` module that handles this properly, but not all files are using it correctly.
> 
> ## Goals
> 
> 1. Remove redundant fallback logic from all consumer files
> 2. Add a new helper function to centralize error handling
> 3. Update all files to use the centralized discovery mechanism
> 4. Ensure consistent formatting and error messages across all files
> 
> ## Current Problems
> 
> - **Duplicated fallback code**: Three files have identical 8-line fallback logic that's redundant
> - **Hardcoded version**: `VsMocks.fs` still hardcodes `VS170COMNTOOLS` instead of using centralized discovery
> - **Inconsistent error messages**: Different files fail with different messages
> - **Poor diagnostics**: Error messages don't explain what discovery methods were tried
> 
> ## Files to Modify
> 
> ### 1. `tests/FSharp.Test.Utilities/VSInstallDiscovery.fs`
> 
> Add a new helper function at the end of the file (after the existing `getVSInstallDirWithLogging` function):
> 
> ```fsharp
>     /// 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."
> ```
> 
> This should be added after line 128 (after the `getVSInstallDirWithLogging` function).
> 
> ### 2. `vsintegration/tests/UnitTests/AssemblyResolver.fs`
> 
> Replace the entire file content with:
> 
> ```fsharp
> 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<CultureInfo>
>                             name.Version <- Unchecked.defaultof<Version>
>                             Some name
>                         else
>                             None
>                     with _ ->
>                         None)
> 
>             match found () with
>             | None -> Unchecked.defaultof<Assembly>
>             | Some name -> Assembly.Load(name))
> ```
> 
> **Key changes from original:**
> - Removed lines 11-24 (redundant fallback logic)
> - Changed line 11 from `match tryGetVSInstallDir()...` to `let vsInstallDir = getVSInstallDirOrFail ()`
> - Changed `fun h args ->` to `fun _ args ->` (unused parameter)
> - Removed extra spaces in `(probingPaths )` → `probingPaths`
> - Consistent formatting
> 
> ### 3. `vsintegration/tests/FSharp.Editor.Tests/Helpers/AssemblyResolver.fs`
> 
> Replace the entire file content with:
> 
> ```fsharp
> // Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
> 
> 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<CultureInfo>
>                             name.Version <- Unchecked.defaultof<Version>
>                             Some name
>                         else
>                             None
>                     with _ ->
>                         None)
> 
>             match found () with
>             | None -> Unchecked.defaultof<Assembly>
>             | Some name -> Assembly.Load(name))
> ```
> 
> **Key changes from original:**
> - Removed lines 13-28 (redundant fallback logic)
> - Changed line 13 from `match tryGetVSInstallDir()...` to `let vsInstallDir = getVSInstallDirOrFail ()`
> - Changed `fun h args ->` to `fun _ args ->` (unused parameter)
> - Consistent formatting
> 
> ### 4. `vsintegration/tests/Salsa/VsMocks.fs`
> 
> Find the `module internal VsActual` section (around line 1633) and update the `vsInstallDir` definition. 
> 
> **Current code (lines ~1633-1653):**
> ```fsharp
> module internal VsActual = 
>     open System.Reflection
>     open System.IO
>     open System.ComponentModel.Composition.Hosting
>     open System.ComponentModel.Composition.Primitives
>     open Microsoft.VisualStudio.Text
>     open Microsoft.VisualStudio.Threading
> 
>     type TestExportJoinableTaskContext () =
>         static let jtc = new JoinableTaskContext()
> 
>         [<System.ComponentModel.Composition.Export(typeof<JoinableTaskContext>)>]
>         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, "..")
> ```
> 
> **Replace with:**
> ```fsharp
> module internal VsActual = 
>     open System.Reflection
>     open System.IO
>     open System.ComponentModel.Composition.Hosting
>     open System.ComponentModel.Composition.Primitives
>     open Microsoft.VisualStudio.Text
>     open Microsoft.VisualStudio.Threading
>     open FSharp.Test.VSInstallDiscovery
> 
>     type TestExportJoinableTaskContext () =
>         static let jtc = new JoinableTaskContext()
> 
>         [<System.ComponentModel.Composition.Export(typeof<JoinableTaskContext>)>]
>         member public _.JoinableTaskContext : JoinableTaskContext = jtc
> 
>     let vsInstallDir = getVSInstallDirOrFail ()
> ```
> 
> **Key changes:**
> - Add `open FSharp.Test.VSInstallDiscovery` to the module opens
> - Replace the entire `vsInstallDir` binding (lines with hardcoded `VS170COMNTOOLS`) with single line: `let vsInstallDir = getVSInstallDirOrFail ()`
> 
> ## Benefits of These Changes
> 
> ✅ **Single source of truth**: All VS version environment variables (`VS180COMNTOOLS`, `VS170COMNTOOLS`, etc.) are only referenced in `VSInstallDiscovery.fs`
> 
> ✅ **Automatic forward compatibility**: When VS 2026 ships, only the central list needs updating
> 
> ✅ **Better error messages**: Users now see which discovery methods were tried (VSAPPIDDIR, VS*COMNTOOLS, vswhere.exe)
> 
> ✅ **Reduced code duplication**: Removed ~40 lines of redundant fallback logic across files
> 
> ✅ **Consistent behavior**: All test infrastructure uses the same discovery logic with same priority order
> 
> ## Testing
> 
> After making these changes:
> 1. Build the solution to ensure no compilation errors
> 2. Run the existing tests to ensure VS installation discovery still works
> 3. The discovery priority order remains: VSAPPIDDIR → VS*COMNTOOLS (trying highest version first) → vswhere.exe
> 
> ## Notes for Implementation
> 
> - The `VSInstallDiscovery.fs` already has the smart discovery logic - we're just adding one helper function
> - The centralized discovery already tries VSAPPIDDIR first (best when running inside VS), then all VS*COMNTOOLS versions (not just VS170), then vswhere.exe
> - All error handling is now centralized in the new `getVSInstallDirOrFail()` function
> - No behavioral changes - just removing redundant code and improving error messages
> 

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/fsharp/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

[github-actions]

✅ No release notes required

[T-Gro]

@copilot :

Pls rename it to avoid conflicts.

D:\a_work\1\s\tests\FSharp.Test.Utilities\CompilerAssert.fs(32,8): error FS0248: Two modules named 'FSharp.Test.AssemblyResolver' occur in two parts of this assembly [D:\a_work\1\s\tests\FSharp.Test.Utilities\FSharp.Test.Utilities.fsproj::TargetFramework=net472]

D:\a_work\1\s\tests\FSharp.Test.Utilities\VSInstallDiscovery.fs(140,8): error FS0248: Two modules named 'FSharp.Test.AssemblyResolver' occur in two parts of this assembly [D:\a_work\1\s\tests\FSharp.Test.Utilities\FSharp.Test.Utilities.fsproj::TargetFramework=net472]
0 Warning(s)
2 Error(s)

[Copilot]

@copilot :

Pls rename it to avoid conflicts....

Fixed in 86177e3. Renamed the module from AssemblyResolver to VSAssemblyResolver to avoid the conflict with the existing AssemblyResolver module in CompilerAssert.fs.

[T-Gro]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).