Move tests to target .NET 8 #6437

Youssef1313 · 2023-01-17T13:48:12Z

Youssef1313 · 2023-01-17T13:48:45Z

@mavasani I'm not sure whether this will pass CI or will need more work, but wanted to confirm if we want to stay on .NET 8 or we want to use .NET 7?

mavasani · 2023-01-17T14:11:38Z

I think .NET 7 is preferable but tagging others to confirm their preference @buyaa-n @jmarolf @sharwell @stephentoub

Youssef1313 · 2023-01-17T14:14:25Z

I think .NET 7 is preferable but tagging others to confirm their preference @buyaa-n @jmarolf @sharwell @stephentoub

For reference, the .NET SDK got upgraded to .NET 8 by #6409. The PR got created and merged automatically.

codecov · 2023-01-17T14:42:13Z

Codecov Report

Merging #6437 (38fa7f1) into main (913f56f) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6437      +/-   ##
==========================================
+ Coverage   96.41%   96.45%   +0.04%     
==========================================
  Files        1371     1372       +1     
  Lines      319011   321402    +2391     
  Branches    10269    10655     +386     
==========================================
+ Hits       307568   310023    +2455     
+ Misses       8985     8935      -50     
+ Partials     2458     2444      -14

Youssef1313 · 2023-01-17T14:58:15Z

Build is green. In case we want to keep on .NET 8, this should be ready to merge. Otherwise, I'll see if it's easy to get back to .NET 7 without build issues

stephentoub · 2023-01-18T04:24:54Z

I think .NET 7 is preferable but tagging others to confirm their preference

Personally, I'd stick with .NET 7 for now until we're further along in .NET 8.

Youssef1313 · 2023-01-18T08:15:29Z

~~This needs #6441 to be merged first.~~

Youssef1313 · 2023-01-19T14:30:01Z

@mavasani Can we merge this?

mavasani · 2023-01-19T14:36:27Z

@stephentoub are you fine with this given the comments in #6441?

mavasani · 2023-01-31T05:43:28Z

@Youssef1313 checking out your branch locally and attempting to run unit tests in VS2022 gives the following error for every test:

Microsoft.CodeAnalysis.Analyzers.UnitTests.MetaAnalyzers.ReleaseTrackingAnalyzerTests.TestInvalidEntryDiagnosticAsync(entry: "Id1 | `<Undetected>` | Warning |", hasUndetectedField: True)
   Source: ReleaseTrackingAnalyzerTests.cs line 614
   Duration: 372 ms

  Message: 
NuGet.Resolver.NuGetResolverInputException : Unable to find package 'Microsoft.CodeAnalysis'. Existing packages must be restored before performing an install or update.

  Stack Trace: 
PackageResolver.Resolve(PackageResolverContext context, CancellationToken token)
ReferenceAssemblies.ResolveCoreAsync(String language, CancellationToken cancellationToken) line 272
ReferenceAssemblies.ResolveAsync(String language, CancellationToken cancellationToken) line 197
ReferenceAssemblies.ResolveAsync(String language, CancellationToken cancellationToken) line 174
AnalyzerTest`1.CreateSolutionAsync(ProjectId projectId, EvaluatedProjectState projectState, CancellationToken cancellationToken) line 1235
AnalyzerTest`1.CreateProjectImplAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, CancellationToken cancellationToken) line 1050
AnalyzerTest`1.CreateProjectAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, CancellationToken cancellationToken) line 1029
AnalyzerTest`1.GetSolutionAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, IVerifier verifier, CancellationToken cancellationToken) line 1007
AnalyzerTest`1.GetSortedDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, ImmutableArray`1 analyzers, IVerifier verifier, CancellationToken cancellationToken) line 890
AnalyzerTest`1.VerifyDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, DiagnosticResult[] expected, IVerifier verifier, CancellationToken cancellationToken) line 273
CodeFixTest`1.RunImplAsync(CancellationToken cancellationToken) line 304
AnalyzerTest`1.RunAsync(CancellationToken cancellationToken) line 181
ReleaseTrackingAnalyzerTests.VerifyCSharpAsync(String source, String shippedText, String unshippedText, DiagnosticResult[] expected) line 1021
ReleaseTrackingAnalyzerTests.TestInvalidEntryDiagnosticAsync(String entry, Boolean hasUndetectedField) line 638
--- End of stack trace from previous location ---

Youssef1313 · 2023-01-31T05:57:49Z

@mavasani Does deleting %TEMP%/test-packages help?

mavasani · 2023-01-31T06:30:10Z

@mavasani Does deleting %TEMP%/test-packages help?

Nope, same issue.

Youssef1313 · 2023-01-31T06:36:12Z

@sharwell Any idea?

mavasani · 2023-01-31T10:09:15Z

@Youssef1313 I have identified the root cause for this issue. It's primarily due to the hardcoding of the Microsoft.CodeAnalysis package at below places:

roslyn-analyzers/src/Test.Utilities/AdditionalMetadataReferences.cs

Line 18 in e516933

    
           .AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis.Workspaces.Common", "3.0.0")));

roslyn-analyzers/src/Test.Utilities/AdditionalMetadataReferences.cs

Line 112 in e516933

    
           referenceAssemblies = referenceAssemblies.AddPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.CodeAnalysis", "3.0.0")));

I am going to push the required fix to your branch. I'll also open a tracking issue for us to figure out some way to avoid this version hardcoding in source and instead derive this version from MicrosoftCodeAnalysisVersion property or at least have a comment in Versions.props to update these hardcoded values when MicrosoftCodeAnalysisVersion is bumped.

Youssef1313 · 2023-01-31T10:13:36Z

Strange that this is only happening after moving to .NET 8.
I also don't understand why these versions have to match. What if I want to write a test for old version of MS.CA?

mavasani · 2023-01-31T10:17:13Z

Strange that this is only happening after moving to .NET 8.

It also happens if you try to run the tests with net472 as active project context. We have just delayed fixing this issue.

I also don't understand why these versions have to match. What if I want to write a test for old version of MS.CA?

We would need some way for test infrastructure to be able to restore the NuGet package with specified version. I don't know if test framework supports this.

Btw, moving to newer version of Microsoft.CodeAnalysis leads to whole bunch of new test failures, which all seem legitimate for the newer package. I'll try to see how much I can fix, and if required hand the PR back to you to fix more failures.

mavasani · 2023-01-31T10:21:25Z

What if I want to write a test for old version of MS.CA?

Seems we actually do have tests which start failing when moved to newer version because they are explicitly relying on absence of new types added in later version of MS.CA. I am not sure how to go about fixing these - I am going to let these keep failing inside VS (as the tests still seem to pass from command line) and open a tracking issue to address this problem.

mavasani · 2023-01-31T10:26:33Z

@Youssef1313 So this causing almost half the tests to still fail inside VS. For example, all tests which use ReferenceAssemblies.Net.Net70 are failing with:

System.Collections.Generic.KeyNotFoundException : The given key 'Microsoft.NETCore.App.Ref.7.0.0-preview.7.22375.6' was not present in the dictionary.

  Stack Trace: 
ImmutableDictionary`2.get_Item(TKey key)
ReferenceAssemblies.ResolveCoreAsync(String language, CancellationToken cancellationToken) line 341
ReferenceAssemblies.ResolveAsync(String language, CancellationToken cancellationToken) line 197
ReferenceAssemblies.ResolveAsync(String language, CancellationToken cancellationToken) line 174
AnalyzerTest`1.CreateSolutionAsync(ProjectId projectId, EvaluatedProjectState projectState, CancellationToken cancellationToken) line 1235
AnalyzerTest`1.CreateProjectImplAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, CancellationToken cancellationToken) line 1050
AnalyzerTest`1.CreateProjectAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, CancellationToken cancellationToken) line 1029
AnalyzerTest`1.GetSolutionAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, IVerifier verifier, CancellationToken cancellationToken) line 1007
AnalyzerTest`1.GetSortedDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, ImmutableArray`1 analyzers, IVerifier verifier, CancellationToken cancellationToken) line 890
AnalyzerTest`1.VerifyDiagnosticsAsync(EvaluatedProjectState primaryProject, ImmutableArray`1 additionalProjects, DiagnosticResult[] expected, IVerifier verifier, CancellationToken cancellationToken) line 273
CodeFixTest`1.RunImplAsync(CancellationToken cancellationToken) line 304
AnalyzerTest`1.RunAsync(CancellationToken cancellationToken) line 181
ConstantExpectedTests.TestCSAsync(String source, DiagnosticResult[] diagnosticResults) line 1097
ConstantExpectedTests.TestArgumentConstantAsync(String type, String minValue, String maxValue, String value, String expression) line 586
--- End of stack trace from previous location ---

mavasani · 2023-01-31T10:28:53Z

@Youssef1313 Feel free to take a stab at this if you'd like. Seems like there many follow-up issues with the testing library to be resolved here.

Meanwhile, I would suggest that we remove the net472 target for test projects, as they fail inside VS and lead to confusion. Tests with netcoreapp31 target are passing fine inside VS as well as command line, so that should probably suffice.

Youssef1313 · 2023-01-31T10:43:16Z

@mavasani Maybe this is related to sources we have in NuGet.config?

I think the test framework might be trying to restore from our root NuGet.config file. Feeds there might not be having packages with requested versions? Not sure if that is the case. Almost guessing.

I can take a closer look on the weekend.

sharwell · 2023-01-31T14:20:44Z

We would need some way for test infrastructure to be able to restore the NuGet package with specified version.

It already does this (it's the only option it supports).

I think the test framework might be trying to restore from our root NuGet.config file.

It's using NuGet.org as the only source unless something inside the test overrides it.

Youssef1313 · 2023-02-19T13:39:42Z

@mavasani I rebased on latest main and all Microsoft.CodeAnalysis.NetAnalyzers.UnitTests tests pass in VS:

I haven't tried before the rebase though. Could you re-test?

Youssef1313 · 2023-03-30T18:39:00Z

@mavasani @stephentoub Could we get this merged?

Youssef1313 · 2023-03-31T08:28:43Z

Since the last commit is quite old, I'm closing and re-opening to start a new build, in case something went in between that fails the changes here.

eng/GenerateAnalyzerNuspec.targets

mavasani · 2023-04-13T04:33:11Z

/azp run

azure-pipelines · 2023-04-13T04:33:28Z

Azure Pipelines successfully started running 3 pipeline(s).

Youssef1313 force-pushed the net8-tests branch from 41c6704 to 81cc28e Compare January 17, 2023 13:56

Youssef1313 marked this pull request as ready for review January 17, 2023 14:58

Youssef1313 requested a review from a team as a code owner January 17, 2023 14:58

Youssef1313 changed the title ~~Move tests to target .NET 8~~ Move tests to target .NET 7 Jan 18, 2023

Youssef1313 changed the title ~~Move tests to target .NET 7~~ Move tests to target .NET 8 Jan 19, 2023

Youssef1313 self-assigned this Jan 31, 2023

Youssef1313 and others added 2 commits February 19, 2023 15:20

Move tests to target .NET 8

a807978

Disable CA1510 for shared files

701d201

Youssef1313 force-pushed the net8-tests branch from 3b683b7 to 701d201 Compare February 19, 2023 13:39

Youssef1313 requested a review from mavasani February 19, 2023 13:45

Use NetCurrent property

38fa7f1

Youssef1313 force-pushed the net8-tests branch from 108f88f to 38fa7f1 Compare February 19, 2023 13:46

Youssef1313 mentioned this pull request Mar 30, 2023

Allow [CallerArgumnetExpressionAttribute] after CancellationToken #6558

Merged

Youssef1313 closed this Mar 31, 2023

Youssef1313 reopened this Mar 31, 2023

mavasani reviewed Apr 13, 2023

View reviewed changes

eng/GenerateAnalyzerNuspec.targets Show resolved Hide resolved

mavasani approved these changes Apr 13, 2023

View reviewed changes

mavasani enabled auto-merge April 13, 2023 04:33

mavasani merged commit e789611 into dotnet:main Apr 13, 2023

github-actions bot added this to the vNext milestone Apr 13, 2023

Youssef1313 deleted the net8-tests branch April 13, 2023 04:55

Move tests to target .NET 8 #6437

Move tests to target .NET 8 #6437

Uh oh!

Conversation

Youssef1313 commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 commented Jan 17, 2023

Uh oh!

mavasani commented Jan 17, 2023

Uh oh!

Youssef1313 commented Jan 17, 2023

Uh oh!

codecov bot commented Jan 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Youssef1313 commented Jan 17, 2023

Uh oh!

stephentoub commented Jan 18, 2023

Uh oh!

Youssef1313 commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 commented Jan 19, 2023

Uh oh!

mavasani commented Jan 19, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

Youssef1313 commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

Youssef1313 commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

Youssef1313 commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

mavasani commented Jan 31, 2023

Uh oh!

Youssef1313 commented Jan 31, 2023

Uh oh!

sharwell commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 commented Feb 19, 2023

Uh oh!

Youssef1313 commented Mar 30, 2023

Uh oh!

Youssef1313 commented Mar 31, 2023

Uh oh!

Uh oh!

mavasani commented Apr 13, 2023

Uh oh!

azure-pipelines bot commented Apr 13, 2023

Uh oh!

Uh oh!

Youssef1313 commented Jan 17, 2023 •

edited

Loading

codecov bot commented Jan 17, 2023 •

edited

Loading

Youssef1313 commented Jan 18, 2023 •

edited

Loading

sharwell commented Jan 31, 2023 •

edited

Loading