Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add project with .Net 8 unit tests #8236

Merged
merged 8 commits into from
Oct 26, 2023
Merged

Conversation

costin-zaharia-sonarsource
Copy link
Member

@costin-zaharia-sonarsource costin-zaharia-sonarsource commented Oct 20, 2023

Fixes #8171

I've tested the coverage import with a separate PR that I had to remove. Here is how it looks on sonarcloud:
image
and this is the build: https://dev.azure.com/sonarsource/DotNetTeam%20Project/_build/results?buildId=79428

Comment on lines +11 to +13
[TestMethod]
public void SelfAssignment() =>
builderCS.AddPaths("SelfAssignment.cs").WithOptions(ParseOptionsHelper.FromCSharp12).Verify();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to demonstrate that we can add .Net 8 unit tests on this project.

@@ -203,13 +209,19 @@ stages:

- powershell: |
cd analyzers
& dotnet test -f $(FrameworkMoniker) -c $(BuildConfiguration) -l trx /p:AltCover=true,AltCoverForce=true,AltCoverVisibleBranches=true,AltCoverAssemblyFilter='Moq|Humanizer|AltCover|Microsoft.VisualStudio.TestPlatform.*|.*Test',AltCoverPathFilter='SonarAnalyzer\.CFG\\ShimLayer|SonarAnalyzer\.ShimLayer\.CodeGeneration',AltCoverAttributeFilter='ExcludeFromCodeCoverage',AltCoverReport=coverage/coverage.xml
if ("$(FrameworkMoniker)" -eq "net8.0-windows") {
& dotnet test "tests\SonarAnalyzer.Net8.UnitTest\SonarAnalyzer.Net8.UnitTest.csproj" -f $(FrameworkMoniker) -c $(BuildConfiguration) -l trx --results-directory $(UnitTestResultsPath) /p:AltCover=true,AltCoverForce=true,AltCoverVisibleBranches=true,AltCoverAssemblyFilter='Moq|Humanizer|AltCover|Microsoft.VisualStudio.TestPlatform.*|.*Test',AltCoverPathFilter='SonarAnalyzer\.CFG\\ShimLayer|SonarAnalyzer\.ShimLayer\.CodeGeneration',AltCoverAttributeFilter='ExcludeFromCodeCoverage',AltCoverReport=$(CoveragePath)/coverage.$(FrameworkMoniker).xml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the target framework parameter is provided, all projects need to support that version. Since this is not possible with .net 8, we have to specify the project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may miss context here, but I'm not sure if we should really do this. What is the future prospect of this change? Are we going to revert it or is this permanent? If we are going to revert it, how do we make sure the new UTs written here are easily movable to the main UT project when we revert? We need to keep a couple of things in sync to do so:

  • namespaces
  • global using

We also need some follow-up issues that describe how we want to evolve this solution in the future.

<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<!-- `-windows` is required in order to be able to reference SonarAnalyzer.UnitTests -->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<!-- `-windows` is required in order to be able to reference SonarAnalyzer.UnitTests -->
<!-- `-windows` is required in order to be able to reference SonarAnalyzer.UnitTests to resolve e.g. WinForms references -->

Comment on lines 213 to 217
& dotnet test "tests\SonarAnalyzer.Net8.UnitTest\SonarAnalyzer.Net8.UnitTest.csproj" -f $(FrameworkMoniker) -c $(BuildConfiguration) -l trx --results-directory $(UnitTestResultsPath) /p:AltCover=true,AltCoverForce=true,AltCoverVisibleBranches=true,AltCoverAssemblyFilter='Moq|Humanizer|AltCover|Microsoft.VisualStudio.TestPlatform.*|.*Test',AltCoverPathFilter='SonarAnalyzer\.CFG\\ShimLayer|SonarAnalyzer\.ShimLayer\.CodeGeneration',AltCoverAttributeFilter='ExcludeFromCodeCoverage',AltCoverReport=$(CoveragePath)/coverage.$(FrameworkMoniker).xml
}
else
{
& dotnet test "tests\SonarAnalyzer.UnitTest\SonarAnalyzer.UnitTest.csproj" -f $(FrameworkMoniker) -c $(BuildConfiguration) -l trx --results-directory $(UnitTestResultsPath) /p:AltCover=true,AltCoverForce=true,AltCoverVisibleBranches=true,AltCoverAssemblyFilter='Moq|Humanizer|AltCover|Microsoft.VisualStudio.TestPlatform.*|.*Test',AltCoverPathFilter='SonarAnalyzer\.CFG\\ShimLayer|SonarAnalyzer\.ShimLayer\.CodeGeneration',AltCoverAttributeFilter='ExcludeFromCodeCoverage',AltCoverReport=$(CoveragePath)/coverage.xml

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two cmd lines only differ in the csproj and the "Report" parameters. Consider using a variable for the common parts.

if ("$(FrameworkMoniker)" -eq "net8.0-windows") {
& dotnet test "tests\SonarAnalyzer.Net8.UnitTest\SonarAnalyzer.Net8.UnitTest.csproj" -f $(FrameworkMoniker) -c $(BuildConfiguration) -l trx --results-directory $(UnitTestResultsPath) /p:AltCover=true,AltCoverForce=true,AltCoverVisibleBranches=true,AltCoverAssemblyFilter='Moq|Humanizer|AltCover|Microsoft.VisualStudio.TestPlatform.*|.*Test',AltCoverPathFilter='SonarAnalyzer\.CFG\\ShimLayer|SonarAnalyzer\.ShimLayer\.CodeGeneration',AltCoverAttributeFilter='ExcludeFromCodeCoverage',AltCoverReport=$(CoveragePath)/coverage.$(FrameworkMoniker).xml
}
else

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also run the "normal" tests in .Net 8.

Suggested change
else

@costin-zaharia-sonarsource
Copy link
Member Author

What is the future prospect of this change? Are we going to revert it or is this permanent?

Hi @martin-strecker-sonarsource, I've clearly mentioned in the readme file that this is a temporary change.

If we are going to revert it, how do we make sure the new UTs written here are easily movable to the main UT project when we revert?

In the same document I've mentioned that the plan is to extract the validation framework. There is no mention about a revert. The problem in the dotnet APIs appeared so many times that we might want to separate some of the tests until they change their design to not use nuget libraries.

This is a way to have progress until 14 November when the new version is released and depending on the fixes they do we might adapt.

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@martin-strecker-sonarsource
Copy link
Contributor

Situation in VS without .Net 8 SDK support (non "preview"):

  • The solution loads and "normal" UTs are working
  • The .Net8.UnitTest project can not be build and these UTs are not executable (SonarAnalyzer.Net8.UnitTest: The current Visual Studio version does not support targeting .NET 8.0. Either target .NET 7.0 or lower, or use Visual Studio version 17.8 or higher)
  • On load, VS modifies the package.lock json of the .Net8.UnitTest project

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. See my last comment for the impact for non-preview VS devs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support .NET 8 UTs
3 participants