Skip to content

Move VS language-service logic tests to FSharp.Compiler.Service.Tests#20033

Open
T-Gro wants to merge 4 commits into
mainfrom
tests/salsa-tests-logic
Open

Move VS language-service logic tests to FSharp.Compiler.Service.Tests#20033
T-Gro wants to merge 4 commits into
mainfrom
tests/salsa-tests-logic

Conversation

@T-Gro

@T-Gro T-Gro commented Jul 5, 2026

Copy link
Copy Markdown
Member

The Visual Studio Salsa unit tests exercised compiler-service logic — completion, quick info, parameter info, go-to-definition, and diagnostics — but only ran on Windows through a mock VS harness. This ports that coverage to the cross-platform FSharp.Compiler.Service.Tests, so it runs on every target the compiler does and no longer depends on Visual Studio.

The ports are behaviour-preserving: each assertion moves onto the equivalent FCS API. The legacy suite keeps only the tests that genuinely exercise Visual Studio integration (project-system, editor events, solution lifecycle).

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

✅ No release notes required

@T-Gro T-Gro force-pushed the tests/salsa-tests-logic branch 3 times, most recently from 2b223cb to b434d50 Compare July 7, 2026 13:22
@T-Gro T-Gro marked this pull request as ready for review July 8, 2026 13:21
@T-Gro T-Gro requested a review from a team as a code owner July 8, 2026 13:21
@T-Gro T-Gro requested a review from abonie July 8, 2026 13:22
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Jul 8, 2026
T-Gro and others added 4 commits July 10, 2026 16:59
Port completion, quick info, parameter info, go-to-definition, and
diagnostics coverage from the Windows-only VS Salsa suite to the
cross-platform FSharp.Compiler.Service.Tests. The legacy suite keeps only
the tests that genuinely exercise Visual Studio integration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Info helper

- ScriptDiagnostics closure: use assumeDotNetFramework=false/useSdkRefs=true on coreclr so the script closure resolves FSharp.Core where no .NET Framework exists (Linux/Mac/Win-transparent).
- ScriptOptions SurfaceOrderOfHashes: guard #if !NETCOREAPP; it asserts desktop-GAC assemblies (System.Runtime.Remoting/System.Transactions) that cannot resolve on a Core-only host.
- Legacy ParameterInfo: delete the now-callerless TestSystematicParameterInfo helper; its wrapper tests were migrated to FCS and a leftover [<Fact>] on the parameterized member failed discovery (0ms).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…the transparent compiler

The migrated legacy test only checked that the resolvable reference produced no squiggle; the whole-closure assertClosureNoDiagnostics over-translated it. The transparent compiler surfaces the deliberately-missing reference as a warning (the classic one does not), so filter that expected warning before asserting none remain.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reference/#load script checks funnelled through a shared "Test.fsx" identity, so the .NET Framework FCS checker's filename-keyed script-closure cache leaked one test's resolved/failed references and their diagnostics into another under concurrency (empty #r tooltips; phantom "'' is not a valid assembly name" squiggles). Route those checks through a unique script identity via getParseAndCheckResultsUniqueName. Plain-code tests keep the fast shared path.

Remove 13 tests whose asserted behaviour is already covered: duplicates within the migrated set (tooltip PriorityQueue/exception/module-alias, gotodef record fields, paraminfo single-arg/generic-location, an if-clause completion that also lives in the completion suite) and two diagnostics covered by executed ComponentTests (FS0010 negative enum literal, FS0433 entrypoint-not-last).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro force-pushed the tests/salsa-tests-logic branch from f588e1e to fc3dae3 Compare July 10, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant