Refactor artifacts size tests (dotnet#42447)

Author: ellahathaway

src/SourceBuild/content/eng/pipelines/source-build-sdk-diff-tests.yml

@@ -46,7 +46,6 @@ jobs:
     targetRid: ${{ variables.centOSStreamX64Rid }}
     architecture: x64
     dotnetDotnetRunId: ${{ parameters.dotnetDotnetRunId }}
-    includeArtifactsSize: true
     publishTestResultsPr: true
 
 - template: templates/jobs/sdk-diff-tests.yml

src/SourceBuild/content/eng/pipelines/templates/jobs/sdk-diff-tests.yml

@@ -11,10 +11,6 @@ parameters:
 - name: dotnetDotnetRunId
   type: string
 
-- name: includeArtifactsSize
-  type: boolean
-  default: false
-
 - name: publishTestResultsPr
   type: boolean
   default: false
@@ -131,7 +127,6 @@ jobs:
       /p:SdkTarballPath=$(SdkTarballPath)
       /p:SourceBuiltArtifactsPath=$(SourceBuiltArtifactsPath)
       /p:SmokeTestsWarnOnSdkContentDiffs=false
-      /p:SmokeTestsIncludeArtifactsSizeTests=${{ parameters.includeArtifactsSize }}
       /p:TargetRid=${{ parameters.targetRid }}
       /p:PortableRid=$(Platform)-${{ parameters.architecture }}
     displayName: Run Tests

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/ArtifactsSizeTests.cs

@@ -17,72 +17,55 @@
 
 namespace Microsoft.DotNet.SourceBuild.SmokeTests;
 
-[Trait("Category", "SdkContent")]
 public class ArtifactsSizeTests : SdkTests
 {
-    private const int SizeThresholdPercentage = 25;
-    private static readonly string BaselineFilePath = BaselineHelper.GetBaselineFilePath($"{Config.TargetRid}.txt", nameof(ArtifactsSizeTests));
-    private readonly Dictionary<string, long> Baseline = new();
-    private Dictionary<string, int> FilePathCountMap = new();
-    private StringBuilder Differences = new();
+    private const string SdkType = "sdk";
+    private readonly StringBuilder _differences = new();
+    private readonly List<string> _newExclusions = new List<string>();
+    private readonly Dictionary<string, int> _filePathCountMap = new();
+    private readonly ExclusionsHelper _exclusionsHelper = new ExclusionsHelper("ZeroSizeExclusions.txt", nameof(ArtifactsSizeTests));
+    public static bool IncludeArtifactsSizeTests => !string.IsNullOrWhiteSpace(Config.SdkTarballPath);
 
-    public ArtifactsSizeTests(ITestOutputHelper outputHelper) : base(outputHelper)
-    {
-        if (File.Exists(BaselineFilePath))
-        {
-            string[] baselineFileContent = File.ReadAllLines(BaselineFilePath);
-            foreach (string entry in baselineFileContent)
-            {
-                string[] splitEntry = entry.Split(':', StringSplitOptions.TrimEntries);
-                Baseline[splitEntry[0]] = long.Parse(splitEntry[1]);
-            }
-        }
-        else
-        {
-            Assert.Fail($"Baseline file `{BaselineFilePath}' does not exist. Please create the baseline file then rerun the test.");
-        }
-    }
+    public ArtifactsSizeTests(ITestOutputHelper outputHelper) : base(outputHelper) {}
 
-    [ConditionalFact(typeof(Config), nameof(Config.IncludeArtifactsSizeTests))]
-    public void CompareArtifactsToBaseline()
+    [ConditionalFact(typeof(ArtifactsSizeTests), nameof(IncludeArtifactsSizeTests))]
+    public void CheckZeroSizeArtifacts()
     {
-        Assert.False(string.IsNullOrWhiteSpace(Config.SourceBuiltArtifactsPath));
-        Assert.False(string.IsNullOrWhiteSpace(Config.SdkTarballPath));
+        ProcessTarball(Config.SdkTarballPath!, SdkType);
 
-        var tarEntries = ProcessSdkAndArtifactsTarballs();
-        ScanForDifferences(tarEntries);
-        UpdateBaselineFile();
+        _exclusionsHelper.GenerateNewBaselineFile(updatedFileTag: null, _newExclusions);
 
-        // Must wait to report differences until after the baseline file is updated else a failure
-        // will cause the baseline file to not be updated.
+        // Wait to report differences until after the baseline file is updated. 
+        // Else a failure will cause the baseline file to not be updated.
         ReportDifferences();
     }
 
-    private Dictionary<string, long> ProcessSdkAndArtifactsTarballs()
+    private void ProcessTarball(string tarballPath, string type)
     {
         string tempTarballDir = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
         Directory.CreateDirectory(tempTarballDir);
 
-        Utilities.ExtractTarball(Config.SdkTarballPath!, tempTarballDir, OutputHelper);
-        Utilities.ExtractTarball(Config.SourceBuiltArtifactsPath!, tempTarballDir, OutputHelper);
+        Utilities.ExtractTarball(tarballPath, tempTarballDir, OutputHelper);
 
-        Dictionary<string, long> tarEntries = Directory.EnumerateFiles(tempTarballDir, "*", SearchOption.AllDirectories)
-            .Where(filePath => !filePath.Contains("SourceBuildReferencePackages"))
-            .Select(filePath =>
-            {
-                string relativePath = filePath.Substring(tempTarballDir.Length + 1);
-                return (ProcessFilePath(relativePath), new FileInfo(filePath).Length);
-            })
-            .ToDictionary(tuple => tuple.Item1, tuple => tuple.Item2);
+        var newZeroSizedFiles = Directory
+            .EnumerateFiles(tempTarballDir, "*", SearchOption.AllDirectories)
+            .Where(filePath => new FileInfo(filePath).Length == 0)
+            .Select(filePath => ProcessFilePath(tempTarballDir, filePath))
+            .Where(processedPath => !_exclusionsHelper.IsFileExcluded(processedPath, type));
 
-        Directory.Delete(tempTarballDir, true);
+        foreach (string file in newZeroSizedFiles)
+        {
+            _newExclusions.Add($"{file}|{type}");
+            TrackDifference($"{file} is 0 bytes.");
+        }
 
-        return tarEntries;
+        Directory.Delete(tempTarballDir, true);
     }
 
-    private string ProcessFilePath(string originalPath)
+    private string ProcessFilePath(string relativeTo, string originalPath)
     {
-        string result = BaselineHelper.RemoveRids(originalPath);
+        string relativePath = Path.GetRelativePath(relativeTo, originalPath);
+        string result = BaselineHelper.RemoveRids(relativePath);
         result = BaselineHelper.RemoveVersions(result);
 
         return AddDifferenciatingSuffix(result);
@@ -111,8 +94,8 @@ private string AddDifferenciatingSuffix(string filePath)
 
         if (matchIndex != -1)
         {
-            int count = FilePathCountMap.TryGetValue(filePath, out count) ? count : 0;
-            FilePathCountMap[filePath] = count + 1;
+            int count = _filePathCountMap.TryGetValue(filePath, out count) ? count : 0;
+            _filePathCountMap[filePath] = count + 1;
 
             if (count > 0)
             {
@@ -123,94 +106,14 @@ private string AddDifferenciatingSuffix(string filePath)
         return filePath;
     }
 
-    private void ScanForDifferences(Dictionary<string, long> tarEntries)
-    {
-        foreach (var entry in tarEntries)
-        {
-            if (!Baseline.TryGetValue(entry.Key, out long baselineBytes))
-            {
-                TrackDifference($"{entry.Key} does not exist in baseline. It is {entry.Value} bytes. Adding it to the baseline file.");
-                Baseline.Add(entry.Key, entry.Value); 
-            }
-            else
-            {
-                CompareFileSizes(entry.Key, entry.Value, baselineBytes);
-            }
-        }
-
-        foreach (var removedFile in Baseline.Keys.Except(tarEntries.Keys))
-        {
-            TrackDifference($"`{removedFile}` is no longer being produced. It was {Baseline[removedFile]} bytes.");
-            Baseline.Remove(removedFile);
-        }
-    }
-
-    private void CompareFileSizes(string filePath, long fileSize, long baselineSize)
-    {
-        // Only update the baseline with breaking differences. Non-breaking differences are file size changes
-        // less than the threshold percentage. This makes it easier to review the breaking changes and prevents
-        // inadvertently allowing small percentage changes to be accepted that can add up to a significant
-        // difference over time.
-        string breakingDifference = string.Empty;
-
-        if (fileSize == 0 && baselineSize != 0)
-        {
-            breakingDifference = $"'{filePath}' is now 0 bytes. It was {baselineSize} bytes.";
-        }
-        else if (fileSize != 0 && baselineSize == 0)
-        {
-            breakingDifference = $"'{filePath}' is no longer 0 bytes. It is now {fileSize} bytes.";
-        }
-        else if (baselineSize != 0 && (((fileSize - baselineSize) / (double)baselineSize) * 100) >= SizeThresholdPercentage)
-        {
-            breakingDifference =
-                $"'{filePath}' increased in size by more than {SizeThresholdPercentage}%. It was originally {baselineSize} bytes and is now {fileSize} bytes.";
-        }
-        else if (baselineSize != 0 && (((baselineSize - fileSize) / (double)baselineSize) * 100) >= SizeThresholdPercentage)
-        {
-            breakingDifference =
-                $"'{filePath}' decreased in size by more than {SizeThresholdPercentage}%. It was originally {baselineSize} bytes and is now {fileSize} bytes.";
-        }
-
-        if (!string.IsNullOrEmpty(breakingDifference))
-        {
-            TrackDifference(breakingDifference);
-            Baseline[filePath] = fileSize;
-        }
-    }
-
-    private void TrackDifference(string difference) => Differences.AppendLine(difference);
+    private void TrackDifference(string difference) => _differences.AppendLine(difference);
 
     private void ReportDifferences()
     {
-        if (Differences.Length > 0)
-        {
-            if (Config.WarnOnSdkContentDiffs)
-            {
-                OutputHelper.LogWarningMessage(Differences.ToString());
-            }
-            else
-            {
-                OutputHelper.WriteLine(Differences.ToString());
-                Assert.Fail("Differences were found in the artifacts sizes.");
-            }
-        }
-    }
-
-    private void UpdateBaselineFile()
-    {
-        try
-        {
-            string actualFilePath = Path.Combine(Config.LogsDirectory, $"Updated{Config.TargetRid}.txt");
-            File.WriteAllLines(
-                actualFilePath,
-                Baseline
-                    .OrderBy(kvp => kvp.Key)
-                    .Select(kvp => $"{kvp.Key}: {kvp.Value}"));
-        }
-        catch (IOException ex)
+        if (_differences.Length > 0)
         {
-            throw new InvalidOperationException($"An error occurred while copying the baselines file: {BaselineFilePath}", ex);
+            OutputHelper.LogWarningMessage(_differences.ToString());
+            Assert.Fail("Differences were found in the artifacts sizes.");
         }
     }
 }

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Config.cs

@@ -21,7 +21,6 @@ internal static class Config
 
     public static string? CustomPackagesPath => (string)AppContext.GetData(ConfigSwitchPrefix + nameof(CustomPackagesPath))!;
     public static bool ExcludeOmniSharpTests => bool.TryParse((string)AppContext.GetData(ConfigSwitchPrefix + nameof(ExcludeOmniSharpTests))!, out bool excludeOmniSharpTests) && excludeOmniSharpTests;
-    public static bool IncludeArtifactsSizeTests => bool.TryParse((string)AppContext.GetData(ConfigSwitchPrefix + nameof(IncludeArtifactsSizeTests))!, out bool includeArtifactsSizeTests) && includeArtifactsSizeTests;
     public static string? LicenseScanPath => (string)AppContext.GetData(ConfigSwitchPrefix + nameof(LicenseScanPath))!;
     public static string? MsftSdkTarballPath => (string)AppContext.GetData(ConfigSwitchPrefix + nameof(MsftSdkTarballPath))!;
     public static string? PoisonReportPath => (string)AppContext.GetData(ConfigSwitchPrefix + nameof(PoisonReportPath))!;

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/ExclusionsHelper.cs

@@ -58,7 +58,12 @@ internal bool IsFileExcluded(string filePath, string suffix = NullSuffix)
             (suffix != NullSuffix && CheckAndRemoveIfExcluded(filePath, NullSuffix));
     }
 
-    internal void GenerateNewBaselineFile(string? updatedFileTag = null)
+    /// <summary>
+    /// Generates a new baseline file with the exclusions that were used during the test run.
+    /// <param name="updatedFileTag">Optional tag to append to the updated file name.</param>
+    /// <param name="additionalLines">Optional additional lines to append to the updated file.</param>
+    /// </summary>
+    internal void GenerateNewBaselineFile(string? updatedFileTag = null, List<string>? additionalLines = null)
     {
         string exclusionsFilePath = BaselineHelper.GetBaselineFilePath(_exclusionsFileName, _baselineSubDir);
 
@@ -68,6 +73,11 @@ internal void GenerateNewBaselineFile(string? updatedFileTag = null)
             .Select(line => UpdateExclusionsLine(line))
             .Where(line => line is not null);
 
+        if (additionalLines is not null)
+        {
+            newLines = newLines.Concat(additionalLines);
+        }
+
         string updatedFileName = updatedFileTag is null
             ? $"Updated{_exclusionsFileName}"
             : $"Updated{Path.GetFileNameWithoutExtension(_exclusionsFileName)}.{updatedFileTag}{Path.GetExtension(_exclusionsFileName)}";

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj

@@ -68,9 +68,6 @@
       <RuntimeHostConfigurationOption Include="$(MSBuildProjectName).ExcludeOmniSharpTests">
         <Value>$(SmokeTestsExcludeOmniSharpTests)</Value>
       </RuntimeHostConfigurationOption>
-      <RuntimeHostConfigurationOption Include="$(MSBuildProjectName).IncludeArtifactsSizeTests">
-        <Value>$(SmokeTestsIncludeArtifactsSizeTests)</Value>
-      </RuntimeHostConfigurationOption>
       <RuntimeHostConfigurationOption Include="$(MSBuildProjectName).LicenseScanPath">
         <Value>$(SmokeTestsLicenseScanPath)</Value>
       </RuntimeHostConfigurationOption>

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/README.md

@@ -11,7 +11,6 @@ Optional msbuild properties:
 - MsftSdkTarballPath
 - SmokeTestsCustomSourceBuiltPackagesPath
 - SmokeTestsExcludeOmniSharpTests
-- SmokeTestsIncludeArtifactsSizeTests
 - SmokeTestsLicenseScanPath
 - SmokeTestsPrereqsPath
 - SmokeTestsWarnOnLicenseScanDiffs

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/assets/ArtifactsSizeTests/ZeroSizeExclusions.txt

@@ -0,0 +1,20 @@
+# Contains the list of files to be excluded from artifact size tests.
+#
+# Format
+#   Exclude the path entirely: 
+#     <path> [# comment]
+#   Exclude a path from a specific artifact:
+#     <path>|{sdk} [# comment]
+#   sdk        = source-built SDK
+#
+# '*' in exclusions match zero or more characters.
+# '*' will match files and directory names but it will not match separator characters.
+# '/' will be evaluated as '/**' if it is the last character.
+#
+# Examples
+# 'folder/*' matches all files and directories in 'folder/'. It will not match 'folder/abc/def'
+# 'folder/' is equivalent to 'folder/**. It matches 'folder/', 'folder/abc', and 'folder/abc/def/'
+
+metadata/workloads/x.y.z/userlocal|sdk
+packs/runtime.banana-rid.Microsoft.DotNet.ILCompiler/x.y.z/sdk/nonportable.txt|sdk
+sdk/x.y.z/Microsoft/Microsoft.NET.Build.Extensions/net471/_._|sdk