Update to latest Xunit and Moq, fix test bugs, stop suppressing analyzers

.editorconfig

@@ -1,39 +0,0 @@
-[*.cs]
-
-# Suppress performance, usage, naming, maintainability, style diagnostics.
-dotnet_analyzer_diagnostic.category-design.severity = none
-dotnet_analyzer_diagnostic.category-globalization.severity = none
-dotnet_analyzer_diagnostic.category-naming.severity = none
-dotnet_analyzer_diagnostic.category-performance.severity = none
-dotnet_analyzer_diagnostic.category-reliability.severity = none
-dotnet_analyzer_diagnostic.category-security.severity = warning
-dotnet_analyzer_diagnostic.category-usage.severity = none
-dotnet_analyzer_diagnostic.category-Maintainability.severity = none
-dotnet_analyzer_diagnostic.category-Style.severity = none
-
-# Addtional suppress to be compatible with different version among VS2019
-
-# CA1001: Types that own disposable fields should be disposable
-dotnet_diagnostic.CA1001.severity = none
-
-# CA1063: Implement IDisposable Correctly
-dotnet_diagnostic.CA1063.severity = none
-
-# CA1033: Interface methods should be callable by child types
-dotnet_diagnostic.CA1033.severity = none
-
-# CA2229: Implement serialization constructors
-dotnet_diagnostic.CA2229.severity = none
-
-# CA2200: Rethrow to preserve stack details.
-dotnet_diagnostic.CA2200.severity = none
-
-# CA1065: Do not raise exceptions in unexpected locations
-dotnet_diagnostic.CA1065.severity = none
-
-# CA2214: Do not call overridable methods in constructors
-dotnet_diagnostic.CA2214.severity = none
-
-# Suppressions we want to reset
-dotnet_diagnostic.CA2017.severity = warning
-dotnet_diagnostic.IDE0005.severity = default # unnecessary usings

.nuget/packages.config

@@ -3,5 +3,4 @@
   <package id="MicroBuild.Core" version="0.3.0" />
   <package id="Microsoft.CodeAnalysis.BinSkim" version="1.3.6" />
   <package id="xunit.runner.console" version="2.1.0" />
-  <package id="xunit.runner.visualstudio" version="2.1.0" />
 </packages>

Directory.Build.props

@@ -1,12 +1,14 @@
 <Project>
   <PropertyGroup>
+    <LangVersion>9.0</LangVersion>
+
     <NuGetClientPackageVersion>6.9.1</NuGetClientPackageVersion>
     <ServerCommonPackageVersion>2.120.0</ServerCommonPackageVersion>
     <NuGetJobsPackageVersion>4.3.0-agr-gal-stsdk-9768098</NuGetJobsPackageVersion>
   </PropertyGroup>
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers">
-      <Version>7.0.3</Version>
+      <Version>8.0.0</Version>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

build.ps1

@@ -101,7 +101,8 @@ Invoke-BuildStep 'Removing .editorconfig file in NuGetGallery' { Remove-Editorco
 
 Invoke-BuildStep 'Building solution' { 
     $SolutionPath = Join-Path $PSScriptRoot "NuGetGallery.sln"
-    Build-Solution -Configuration $Configuration -BuildNumber $BuildNumber -SolutionPath $SolutionPath -SkipRestore:$SkipRestore -MSBuildProperties "/p:MvcBuildViews=true" `
+    $MvcBuildViews = $Configuration -eq "release"
+    Build-Solution -Configuration $Configuration -BuildNumber $BuildNumber -SolutionPath $SolutionPath -SkipRestore:$SkipRestore -MSBuildProperties "/p:MvcBuildViews=$MvcBuildViews" `
 } `
 -ev +BuildErrors
 

src/GitHubVulnerabilities2Db/Gallery/GalleryDbVulnerabilityWriter.cs

@@ -56,8 +56,8 @@ public async Task WriteVulnerabilityAsync(PackageVulnerability vulnerability, bo
             }
             catch (Exception ex)
             {
-                _logger.LogError("Failed to write vulnerability {GitHubAdvisoryUrl}", vulnerability.AdvisoryUrl);
-                throw ex;
+                _logger.LogError(ex, "Failed to write vulnerability {GitHubAdvisoryUrl}", vulnerability.AdvisoryUrl);
+                throw;
             }
         }
     }

src/GitHubVulnerabilities2v3/Extensions/BlobStorageVulnerabilityWriter.cs

@@ -53,7 +53,10 @@ public BlobStorageVulnerabilityWriter(
             _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
 
             _storage = _storageFactory.Create();
-            ((AzureStorage)_storage).CompressContent = _configuration.GzipFileContent;
+            if (_storage is AzureStorage azureStorage)
+            {
+                azureStorage.CompressContent = _configuration.GzipFileContent;
+            }
 
             _firstVulnWrittenTimestamp = DateTimeOffset.MinValue;
             _vulnerabilityDict = new Dictionary<string, List<Advisory>>(StringComparer.OrdinalIgnoreCase);

src/GitHubVulnerabilities2v3/GitHubVulnerabilities2V3.csproj

@@ -93,9 +93,6 @@
       <Name>NuGetGallery.Services</Name>
     </ProjectReference>
   </ItemGroup>
-  <ItemGroup>
-    <Folder Include="Properties\" />
-  </ItemGroup>
   <PropertyGroup>
     <SignPath>..\..\build</SignPath>
     <SignPath Condition="'$(BUILD_SOURCESDIRECTORY)' != ''">$(BUILD_SOURCESDIRECTORY)\build</SignPath>

src/NuGetGallery.Core/Entities/EntitiesContext.cs

@@ -8,6 +8,7 @@
 using System.Data.Entity.Infrastructure;
 using System.Data.Entity.Infrastructure.Annotations;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using NuGet.Services.Entities;
 
@@ -97,14 +98,30 @@ DbSet<T> IReadOnlyEntitiesContext.Set<T>()
             return base.Set<T>();
         }
 
+        public override int SaveChanges()
+        {
+            ThrowIfReadOnly();
+            return base.SaveChanges();
+        }
+
+        public override Task<int> SaveChangesAsync(CancellationToken cancellationToken)
+        {
+            ThrowIfReadOnly();
+            return base.SaveChangesAsync(cancellationToken);
+        }
+
         public override async Task<int> SaveChangesAsync()
+        {
+            ThrowIfReadOnly();
+            return await base.SaveChangesAsync();
+        }
+
+        private void ThrowIfReadOnly()
         {
             if (ReadOnly)
             {
                 throw new ReadOnlyModeException("Save changes unavailable: the gallery is currently in read only mode, with limited service. Please try again later.");
             }
-
-            return await base.SaveChangesAsync();
         }
 
         public void DeleteOnCommit<T>(T entity) where T : class

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -123,7 +123,7 @@ public async Task<JsonResult> AddPackageOwner(AddPackageOwnerViewModel addOwnerD
             string username = addOwnerData.Username;
             string message = addOwnerData.Message;
 
-            if (Regex.IsMatch(username, GalleryConstants.EmailValidationRegex, RegexOptions.None, GalleryConstants.EmailValidationRegexTimeout))
+            if (username is not null && Regex.IsMatch(username, GalleryConstants.EmailValidationRegex, RegexOptions.None, GalleryConstants.EmailValidationRegexTimeout))
             {
                 return Json(new { success = false, message = Strings.AddOwner_NameIsEmail }, JsonRequestBehavior.AllowGet);
             }

test.ps1

@@ -1,11 +1,10 @@
-[CmdletBinding(DefaultParameterSetName='RegularBuild')]
+[CmdletBinding(DefaultParameterSetName = 'RegularBuild')]
 param (
     [ValidateSet("debug", "release")]
     [string]$Configuration = 'debug',
     [int]$BuildNumber
 )
 
-# For TeamCity - If any issue occurs, this script fail the build. - By default, TeamCity returns an exit code of 0 for all powershell scripts, even if they fail
 trap {
     Write-Host "BUILD FAILED: $_" -ForegroundColor Red
     Write-Host "ERROR DETAILS:" -ForegroundColor Red
@@ -16,28 +15,33 @@ trap {
 
 . "$PSScriptRoot\build\common.ps1"
 
-Function Run-Tests {
+Function Invoke-Tests {
     [CmdletBinding()]
     param()
 
     Trace-Log 'Running tests'
 
     $xUnitExe = (Join-Path $PSScriptRoot "packages\xunit.runner.console\tools\xunit.console.exe")
 
-    $TestAssemblies = `
-        "tests\AccountDeleter.Facts\bin\$Configuration\AccountDeleter.Facts.dll", `
-        "tests\GitHubVulnerabilities2Db.Facts\bin\$Configuration\GitHubVulnerabilities2Db.Facts.dll", `
+    $GalleryTestAssemblies = `
+        "tests\AccountDeleter.Facts\bin\$Configuration\net472\AccountDeleter.Facts.dll", `
+        "tests\GitHubVulnerabilities2Db.Facts\bin\$Configuration\net472\GitHubVulnerabilities2Db.Facts.dll", `
         "tests\GitHubVulnerabilities2v3.Facts\bin\$Configuration\GitHubVulnerabilities2v3.Facts.dll", `
-        "tests\NuGet.Services.DatabaseMigration.Facts\bin\$Configuration\NuGet.Services.DatabaseMigration.Facts.dll", `
-        "tests\NuGet.Services.Entities.Tests\bin\$Configuration\NuGet.Services.Entities.Tests.dll", `
+        "tests\NuGet.Services.DatabaseMigration.Facts\bin\$Configuration\net472\NuGet.Services.DatabaseMigration.Facts.dll", `
+        "tests\NuGet.Services.Entities.Tests\bin\$Configuration\net472\NuGet.Services.Entities.Tests.dll", `
         "tests\NuGetGallery.Core.Facts\bin\$Configuration\NuGetGallery.Core.Facts.dll", `
         "tests\NuGetGallery.Facts\bin\$Configuration\NuGetGallery.Facts.dll", `
         "tests\VerifyMicrosoftPackage.Facts\bin\$Configuration\NuGet.VerifyMicrosoftPackage.Facts.dll"
 
     $TestCount = 0
-
-    foreach ($Test in $TestAssemblies) {
-        & $xUnitExe (Join-Path $PSScriptRoot $Test) -xml "Results.$TestCount.xml"
+
+    $GalleryTestAssemblies | ForEach-Object {
+        $TestResultFile = Join-Path $PSScriptRoot "Results.$TestCount.xml"
+        & $xUnitExe (Join-Path $PSScriptRoot $_) -xml $TestResultFile
+        if (-not (Test-Path $TestResultFile)) {
+            Write-Error "The test run failed to produce a result file";
+            exit 1;
+        }
         $TestCount++
     }
 
@@ -56,7 +60,7 @@ Trace-Log "Build #$BuildNumber started at $startTime"
 
 $BuildErrors = @()
 
-Invoke-BuildStep 'Running tests' { Run-Tests } `
+Invoke-BuildStep 'Running tests' { Invoke-Tests } `
     -ev +BuildErrors
 
 Trace-Log ('-' * 60)
@@ -69,7 +73,7 @@ Trace-Log "Time elapsed $(Format-ElapsedTime ($endTime - $startTime))"
 Trace-Log ('=' * 60)
 
 if ($BuildErrors) {
-    $ErrorLines = $BuildErrors | %{ ">>> $($_.Exception.Message)" }
+    $ErrorLines = $BuildErrors | ForEach-Object { ">>> $($_.Exception.Message)" }
     Error-Log "Tests completed with $($BuildErrors.Count) error(s):`r`n$($ErrorLines -join "`r`n")" -Fatal
 }
 

tests/AccountDeleter.Facts/AccountDeleter.Facts.csproj

@@ -10,7 +10,7 @@
     <ProjectReference Include="..\..\src\NuGetGallery.Core\NuGetGallery.Core.csproj" />
     <ProjectReference Include="..\..\src\NuGetGallery.Services\NuGetGallery.Services.csproj" />
 
-    <PackageReference Include="Moq" Version="4.8.2" />
-    <PackageReference Include="xunit" Version="2.5.0" />
+    <PackageReference Include="Moq" Version="4.20.70" />
+    <PackageReference Include="xunit" Version="2.9.0" />
   </ItemGroup>
 </Project>

tests/GitHubVulnerabilities2Db.Facts/GitHubVulnerabilities2Db.Facts.csproj

@@ -9,7 +9,7 @@
     <ProjectReference Include="..\..\src\NuGetGallery.Core\NuGetGallery.Core.csproj" />
     <ProjectReference Include="..\..\src\NuGetGallery.Services\NuGetGallery.Services.csproj" />
 
-    <PackageReference Include="Moq" Version="4.8.2" />
-    <PackageReference Include="xunit" Version="2.5.0" />
+    <PackageReference Include="Moq" Version="4.20.70" />
+    <PackageReference Include="xunit" Version="2.9.0" />
   </ItemGroup>
 </Project>

tests/GitHubVulnerabilities2v3.Facts/GitHubVulnerabilities2v3.Facts.csproj

@@ -7,8 +7,8 @@
     <ProjectGuid>{46A2C2EB-B7DC-4FAB-ABE2-A2CE6118585C}</ProjectGuid>
     <OutputType>Library</OutputType>
     <AppDesignerFolder>Properties</AppDesignerFolder>
-    <RootNamespace>GitHubVulnerabilities2Db.Facts</RootNamespace>
-    <AssemblyName>GitHubVulnerabilities2Db.Facts</AssemblyName>
+    <RootNamespace>GitHubVulnerabilities2v3.Facts</RootNamespace>
+    <AssemblyName>GitHubVulnerabilities2v3.Facts</AssemblyName>
     <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion>
     <FileAlignment>512</FileAlignment>
     <Deterministic>true</Deterministic>
@@ -68,12 +68,8 @@
     </ProjectReference>
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="Moq">
-      <Version>4.8.2</Version>
-    </PackageReference>
-    <PackageReference Include="xunit">
-      <Version>2.5.0</Version>
-    </PackageReference>
+    <PackageReference Include="Moq" Version="4.20.70" />
+    <PackageReference Include="xunit" Version="2.9.0" />
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
 </Project>

tests/NuGet.Services.DatabaseMigration.Facts/NuGet.Services.DatabaseMigration.Facts.csproj

@@ -6,10 +6,9 @@
     <ProjectReference Include="..\..\src\DatabaseMigrationTools\DatabaseMigrationTools.csproj" />
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="Moq" Version="4.8.2" />
-    <PackageReference Include="xunit" Version="2.5.0" />
-    <PackageReference Include="xunit.analyzers" Version="1.2.0" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.0">
+    <PackageReference Include="Moq" Version="4.20.70" />
+    <PackageReference Include="xunit" Version="2.9.0" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2">
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

tests/NuGet.Services.Entities.Tests/NuGet.Services.Entities.Tests.csproj

@@ -6,8 +6,8 @@
     <ProjectReference Include="..\..\src\NuGet.Services.Entities\NuGet.Services.Entities.csproj" />
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="xunit" Version="2.5.0" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.0" >
+    <PackageReference Include="xunit" Version="2.9.0" />
+    <PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" >
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>

tests/NuGetGallery.Core.Facts/Frameworks/FrameworkCompatibilityServiceFacts.cs

@@ -35,7 +35,7 @@ public void UnknownSupportedPackageReturnsSetWithSameFramework()
             var compatible = FrameworkCompatibilityService.GetCompatibleFrameworks(frameworks);
 
             Assert.False(framework.IsUnsupported);
-            Assert.Equal(expected: 1, compatible.Count);
+            Assert.Single(compatible);
             Assert.Contains(framework, compatible);
         }
 
@@ -50,7 +50,7 @@ public void UnsupportedPackageFrameworksReturnsEmptySet(string unsupportedFramew
             var result = FrameworkCompatibilityService.GetCompatibleFrameworks(new List<NuGetFramework>() { unsupportedFramework });
 
             Assert.True(unsupportedFramework.IsUnsupported);
-            Assert.Equal(expected: 0, actual: result.Count);
+            Assert.Empty(result);
         }
 
         [Theory]
@@ -64,7 +64,7 @@ public void PCLPackageFrameworksReturnsEmptySet(string pclFrameworkName)
             var result = FrameworkCompatibilityService.GetCompatibleFrameworks(new List<NuGetFramework>() { portableFramework });
 
             Assert.True(portableFramework.IsPCL);
-            Assert.Equal(expected: 0, actual: result.Count);
+            Assert.Empty(result);
         }
 
         [Theory]

tests/NuGetGallery.Core.Facts/Frameworks/PackageFrameworkCompatibilityFactoryFacts.cs

@@ -142,11 +142,11 @@ public void FrameworksShouldOnlyBeOnASingleProductFrameworkName(string productNa
                 {
                     if (productName.Equals(row.Key))
                     {
-                        Assert.True(row.Value.Any(f => f.Framework == packgeFramework.FrameworkName));
+                        Assert.Contains(row.Value, f => f.Framework == packgeFramework.FrameworkName);
                     }
                     else
                     {
-                        Assert.False(row.Value.Any(f => f.Framework == packgeFramework.FrameworkName));
+                        Assert.DoesNotContain(row.Value, f => f.Framework == packgeFramework.FrameworkName);
                     }
                 }
             }

tests/NuGetGallery.Core.Facts/GitHub/GitHubUsageConfigurationFacts.cs

@@ -57,7 +57,7 @@ public void SingleRepoOneDependency()
                 var gh = GenConfig(expectedRepo);
                 var nupkgInformation = gh.GetPackageInformation("nupkg1");
 
-                Assert.Equal(1, nupkgInformation.Repos.Count);
+                Assert.Single(nupkgInformation.Repos);
                 Assert.Equal(1, nupkgInformation.TotalRepos);
                 Assert.Equal(expectedRepo, nupkgInformation.Repos.First());
             }
@@ -86,11 +86,11 @@ public void OneRepoMultiDependencies()
                 var nupkgInformation1 = gh.GetPackageInformation("nupkg1");
                 var nupkgInformation2 = gh.GetPackageInformation("nupkg2");
 
-                Assert.Equal(1, nupkgInformation1.Repos.Count);
+                Assert.Single(nupkgInformation1.Repos);
                 Assert.Equal(1, nupkgInformation1.TotalRepos);
                 Assert.Equal(expectedRepo, nupkgInformation1.Repos.First());
 
-                Assert.Equal(1, nupkgInformation2.Repos.Count);
+                Assert.Single(nupkgInformation2.Repos);
                 Assert.Equal(1, nupkgInformation2.TotalRepos);
                 Assert.Equal(expectedRepo, nupkgInformation2.Repos.First());
             }
@@ -162,7 +162,7 @@ public void CaseInsensitive()
                 var gh = GenConfig(expectedRepo);
                 var nupkgInformation = gh.GetPackageInformation("NuPkG1");
 
-                Assert.Equal(1, nupkgInformation.Repos.Count);
+                Assert.Single(nupkgInformation.Repos);
                 Assert.Equal(1, nupkgInformation.TotalRepos);
                 Assert.Equal(expectedRepo, nupkgInformation.Repos.First());
             }

tests/NuGetGallery.Core.Facts/Infrastructure/Mail/Messages/PackageAddedMessageFacts.cs

@@ -54,7 +54,7 @@ public void AddsPackageOwnersSubscribedToPackagePushedNotificationToToList()
                 var message = CreateMessage();
                 var recipients = message.GetRecipients();
 
-                Assert.Equal(1, recipients.To.Count);
+                Assert.Single(recipients.To);
                 Assert.Contains(
                     Fakes.PackageOwnerSubscribedToPackagePushedNotification.ToMailAddress(),
                     recipients.To);

tests/NuGetGallery.Core.Facts/Infrastructure/Mail/Messages/PackageValidationTakingTooLongMessageFacts.cs

@@ -45,7 +45,7 @@ public void AddsPackageOwnersSubscribedToPackagePushedNotificationToToList()
                 var message = CreateMessage();
                 var recipients = message.GetRecipients();
 
-                Assert.Equal(1, recipients.To.Count);
+                Assert.Single(recipients.To);
                 Assert.Contains(
                     Fakes.PackageOwnerSubscribedToPackagePushedNotification.ToMailAddress(),
                     recipients.To);