Use locally built CodeAnalysis testing libraries - #85215
Conversation
|
Azure Pipelines: Successfully started running 2 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are straightforward dependency-graph updates with valid project paths/TFM compatibility and no functional code changes identified in the modified files.
Review tier: Lite
Findings: None
What changed in this PR
This PR updates Roslyn test-utility and test projects to use in-repo RoslynSdk/Microsoft.CodeAnalysis.Testing projects (via ProjectReference) instead of consuming the Microsoft.CodeAnalysis.*.Testing NuGet packages. This shifts these tests to build against local sources for the testing libraries, reducing reliance on package publication/update cadence.
Changes:
- Replace
Microsoft.CodeAnalysis.*.TestingPackageReferences in Workspaces test utilities withProjectReferences to the RoslynSdk testing projects. - Update CodeStyle unit test utilities and C# CodeStyle unit tests to reference the same local testing-library projects.
- Align dependency flow so downstream tests can obtain testing-library assemblies through project-to-project build graph rather than NuGet packages.
| File | Description |
|---|---|
| src/Workspaces/CoreTestUtilities/Microsoft.CodeAnalysis.Workspaces.Test.Utilities.csproj | Removes Microsoft.CodeAnalysis.*.Testing package refs and adds project refs to RoslynSdk testing-library projects. |
| src/CodeStyle/CSharp/Tests/Microsoft.CodeAnalysis.CSharp.CodeStyle.UnitTests.csproj | Switches C# CodeFix testing dependency from package to local RoslynSdk project reference. |
| src/CodeStyle/Core/Tests/UnitTestUtilities/Microsoft.CodeAnalysis.CodeStyle.UnitTestUtilities.csproj | Switches C# / VB CodeFix+Refactoring testing dependencies from packages to local RoslynSdk project references. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The dependency changes are consistent across the touched projects, unused references were validated by inspection, and the updated Razor transitive pin matches the referenced testing-library project’s dependency chain.
Review tier: Lite
Findings: None
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The dependency graph and central-package-version adjustments are consistent and appear complete, with no remaining unresolved package-version or reference inconsistencies found in the repo.
Review tier: Lite
Findings: None
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
CopyLocalLockFileAssemblies is disabled for all .NET Framework targets under Microsoft.CodeAnalysis.Testing, which likely breaks running the net472 unit test targets in that subtree.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/RoslynSdk/Microsoft.CodeAnalysis.Testing/Directory.Build.targets — The CopyLocalLockFileAssemblies=false setting is currently applied to all projects in this… |
| </Project> No newline at end of file | ||
|
|
||
| <PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'"> | ||
| <CopyLocalLockFileAssemblies>false</CopyLocalLockFileAssemblies> |
There was a problem hiding this comment.
BuildBoss was flagging multiple writes in .NET Framework consumers of these libraries as package references would bring in System.Text.Json and also the project references to these projects would bring in a copy from their OutputDirectory.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A whitespace-only blank line was introduced in a changed project file and will fail Roslyn’s whitespace conventions/formatting checks.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests.csproj — Line 38 is a whitespace-only blank line. Roslyn’s repo-wide conventions require blank lines to… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/RoslynSdk/Microsoft.CodeAnalysis.Testing/Directory.Build.targets — The CopyLocalLockFileAssemblies=false setting is currently applied to all projects in this… View resolved comment |
| <ProjectReference Include="..\..\..\..\..\Workspaces\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.Workspaces.csproj" /> | ||
|
|
||
| </ItemGroup> |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It broadly changes the dependency graph (project-vs-package references) across multiple areas and can impact build/pack/test behavior in CI in ways that warrant maintainer verification.
Review tier: Lite
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
|
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests.csproj — Line 38 is a whitespace-only blank line. Roslyn’s repo-wide conventions require blank lines to… View comment |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d6a35c4-0f49-4671-b352-8cac98e9eaa4
f2738e6 to
ab115c2
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Preserve the removed NET-specific dispose-pattern coverage or add equivalent coverage.
Review tier: Lite
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
|
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests.csproj — Line 38 is a whitespace-only blank line. Roslyn’s repo-wide conventions require blank lines to… View comment |
Suppressed comments (1)
src/Analyzers/CSharp/Tests/ImplementInterface/ImplementInterfaceCodeFixTests.cs:7249
- This removes the only NET-specific coverage for generating
using System.Diagnostics.CodeAnalysiswhen the dispose-pattern fix implementsIEquatable<int>with an[AllowNull]parameter. The old conditional branch kept that scenario on the modern target while using the framework-compatible signature onnet472; please preserve that branch (or add an equivalent test) so this migration does not silently drop coverage for namespace insertion.
var extraUsing = "";
var equalsMethod = """
public bool Equals(int other)
{
throw new NotImplementedException();
}
""";
Keep ReferenceAssemblies.Default on the .NET Core 3.1 surface and require tests that depend on .NET 10 APIs to opt in explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b6bbeadc-d0a7-4f79-9f55-a54f05246e32
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The broad dependency, framework, and reference-assembly changes warrant final human review.
Review tier: Lite
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
|
src/Razor/src/Compiler/test/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests/Microsoft.NET.Sdk.Razor.SourceGenerators.UnitTests.csproj — Line 38 is a whitespace-only blank line. Roslyn’s repo-wide conventions require blank lines to… View comment |
Suppressed comments (1)
.github/memory/TESTING_STRATEGY.md:33
- This guidance overstates what
ReferenceAssemblies.Defaultselects: the implementation returnsNetFramework.Net472.DefaultforNET472andNetStandard.NetStandard20forNETSTANDARD2_0(src/RoslynSdk/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/ReferenceAssemblies.cs:91-100), not .NET Core 3.1. Qualify the recommendation to modern .NET targets or document the target-dependent behavior so tests targeting the other library TFMs are not given an incorrect contract.
- Analyzer testing-library tests should use `ReferenceAssemblies.Default` for
the stable .NET Core 3.1 reference surface. Use an explicit
`ReferenceAssemblies` value, such as `ReferenceAssemblies.Net.Net100`, when
the scenario intentionally depends on a newer or specific framework API
surface.
Summary
Microsoft.CodeAnalysis.*.Testingdependencies in internal repository projects with local project referenceseng/Packages.propsSystem.Formats.Asn1pin with the locally built testing library dependencyThis removes the package publication/update cycle for testing-library fixes used by internal tests, including the NuGet source-resolution issue observed in build 1587758.
Validation
Microsoft.CodeAnalysis.*.Testingdependencies as project assetsSystem.Text.JsonorSystem.Text.Encodings.WebRemoveUnusedParametersTests: 133 passed onnet10.0AddImportCodeRefactoringTests: 52 passed onnet10.0net10.0net10.0The Visual Basic CodeStyle and Visual Studio source-generator projects target .NET Framework; they were built but cannot be executed on macOS.