Fix deterministic build+source link bug

Documentation/Changelog.md

@@ -7,7 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Fixed
--Attribute exclusion does not work if attribute name does not end with "Attribute" [883](https://github.com/coverlet-coverage/coverlet/issues/883) by https://github.com/bddckr
+-Attribute exclusion does not work if attribute name does not end with "Attribute" [884](https://github.com/coverlet-coverage/coverlet/pull/884) by https://github.com/bddckr  
+-Fix deterministic build+source link bug [895](https://github.com/coverlet-coverage/coverlet/pull/895)
 
 ## Release date 2020-05-30
 ### Packages  

src/coverlet.core/Abstractions/ISourceRootTranslator.cs

@@ -1,7 +1,11 @@
-namespace Coverlet.Core.Abstractions
+using System.Collections.Generic;
+using Coverlet.Core.Helpers;
+
+namespace Coverlet.Core.Abstractions
 {
     internal interface ISourceRootTranslator
     {
         string ResolveFilePath(string originalFileName);
+        IReadOnlyList<SourceRootMapping> ResolvePathRoot(string pathRoot);
     }
 }

src/coverlet.core/Coverage.cs

@@ -3,6 +3,7 @@
 using System.IO;
 using System.Linq;
 using Coverlet.Core.Abstractions;
+using Coverlet.Core.Helpers;
 using Coverlet.Core.Instrumentation;
 
 using Newtonsoft.Json;
@@ -77,7 +78,11 @@ public Coverage(string module,
             _results = new List<InstrumenterResult>();
         }
 
-        public Coverage(CoveragePrepareResult prepareResult, ILogger logger, IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem)
+        public Coverage(CoveragePrepareResult prepareResult,
+                        ILogger logger,
+                        IInstrumentationHelper instrumentationHelper,
+                        IFileSystem fileSystem,
+                        ISourceRootTranslator sourceRootTranslator)
         {
             _identifier = prepareResult.Identifier;
             _module = prepareResult.Module;
@@ -87,6 +92,7 @@ public Coverage(CoveragePrepareResult prepareResult, ILogger logger, IInstrument
             _logger = logger;
             _instrumentationHelper = instrumentationHelper;
             _fileSystem = fileSystem;
+            _sourceRootTranslator = sourceRootTranslator;
         }
 
         public CoveragePrepareResult PrepareModules()
@@ -429,29 +435,33 @@ private string GetSourceLinkUrl(Dictionary<string, string> sourceLinkDocuments,
                 string key = sourceLinkDocument.Key;
                 if (Path.GetFileName(key) != "*") continue;
 
-                string directoryDocument = Path.GetDirectoryName(document);
-                string sourceLinkRoot = Path.GetDirectoryName(key);
-                string relativePath = "";
-
-                // if document is on repo root we skip relative path calculation
-                if (directoryDocument != sourceLinkRoot)
+                IReadOnlyList<SourceRootMapping> rootMapping = _sourceRootTranslator.ResolvePathRoot(key.Substring(0, key.Length - 1));
+                foreach (string keyMapping in rootMapping is null ? new List<string>() { key } : new List<string>(rootMapping.Select(m => m.OriginalPath)))
                 {
-                    if (!directoryDocument.StartsWith(sourceLinkRoot + Path.DirectorySeparatorChar))
-                        continue;
+                    string directoryDocument = Path.GetDirectoryName(document);
+                    string sourceLinkRoot = Path.GetDirectoryName(keyMapping);
+                    string relativePath = "";
 
-                    relativePath = directoryDocument.Substring(sourceLinkRoot.Length + 1);
-                }
+                    // if document is on repo root we skip relative path calculation
+                    if (directoryDocument != sourceLinkRoot)
+                    {
+                        if (!directoryDocument.StartsWith(sourceLinkRoot + Path.DirectorySeparatorChar))
+                            continue;
 
-                if (relativePathOfBestMatch.Length == 0)
-                {
-                    keyWithBestMatch = sourceLinkDocument.Key;
-                    relativePathOfBestMatch = relativePath;
-                }
+                        relativePath = directoryDocument.Substring(sourceLinkRoot.Length + 1);
+                    }
 
-                if (relativePath.Length < relativePathOfBestMatch.Length)
-                {
-                    keyWithBestMatch = sourceLinkDocument.Key;
-                    relativePathOfBestMatch = relativePath;
+                    if (relativePathOfBestMatch.Length == 0)
+                    {
+                        keyWithBestMatch = sourceLinkDocument.Key;
+                        relativePathOfBestMatch = relativePath;
+                    }
+
+                    if (relativePath.Length < relativePathOfBestMatch.Length)
+                    {
+                        keyWithBestMatch = sourceLinkDocument.Key;
+                        relativePathOfBestMatch = relativePath;
+                    }
                 }
             }
 

src/coverlet.core/Helpers/SourceRootTranslator.cs

@@ -19,7 +19,7 @@ internal class SourceRootTranslator : ISourceRootTranslator
         private readonly IFileSystem _fileSystem;
         private readonly Dictionary<string, List<SourceRootMapping>> _sourceRootMapping;
         private const string MappingFileName = "CoverletSourceRootsMapping";
-        private Dictionary<string, string> _resolutionCache;
+        private Dictionary<string, string> _resolutionCacheFiles;
 
         public SourceRootTranslator(ILogger logger, IFileSystem fileSystem)
         {
@@ -76,11 +76,16 @@ private Dictionary<string, List<SourceRootMapping>> LoadSourceRootMapping(string
             return mapping;
         }
 
+        public IReadOnlyList<SourceRootMapping> ResolvePathRoot(string pathRoot)
+        {
+            return _sourceRootMapping.TryGetValue(pathRoot, out List<SourceRootMapping> sourceRootMapping) ? sourceRootMapping.AsReadOnly() : null;
+        }
+
         public string ResolveFilePath(string originalFileName)
         {
-            if (_resolutionCache != null && _resolutionCache.ContainsKey(originalFileName))
+            if (_resolutionCacheFiles != null && _resolutionCacheFiles.ContainsKey(originalFileName))
             {
-                return _resolutionCache[originalFileName];
+                return _resolutionCacheFiles[originalFileName];
             }
 
             foreach (KeyValuePair<string, List<SourceRootMapping>> mapping in _sourceRootMapping)
@@ -92,7 +97,7 @@ public string ResolveFilePath(string originalFileName)
                         string pathToCheck;
                         if (_fileSystem.Exists(pathToCheck = Path.GetFullPath(originalFileName.Replace(mapping.Key, srm.OriginalPath))))
                         {
-                            (_resolutionCache ??= new Dictionary<string, string>()).Add(originalFileName, pathToCheck);
+                            (_resolutionCacheFiles ??= new Dictionary<string, string>()).Add(originalFileName, pathToCheck);
                             _logger.LogVerbose($"Mapping resolved: '{originalFileName}' -> '{pathToCheck}'");
                             return pathToCheck;
                         }

src/coverlet.msbuild.tasks/CoverageResultTask.cs

@@ -96,7 +96,7 @@ public override bool Execute()
                     // Task.Log is teared down after a task and thus the new MSBuildLogger must be passed to the InstrumentationHelper
                     // https://github.com/microsoft/msbuild/issues/5153
                     instrumentationHelper.SetLogger(_logger);
-                    coverage = new Coverage(CoveragePrepareResult.Deserialize(instrumenterStateStream), this._logger, ServiceProvider.GetService<IInstrumentationHelper>(), fileSystem);
+                    coverage = new Coverage(CoveragePrepareResult.Deserialize(instrumenterStateStream), this._logger, ServiceProvider.GetService<IInstrumentationHelper>(), fileSystem, ServiceProvider.GetService<ISourceRootTranslator>());
                 }
 
                 try

test/coverlet.core.tests/Coverage/InstrumenterHelper.cs

@@ -63,7 +63,7 @@ public static CoverageResult GetCoverageResult(string filePath)
             });
             _processWideContainer.GetRequiredService<IInstrumentationHelper>().SetLogger(logger.Object);
             CoveragePrepareResult coveragePrepareResultLoaded = CoveragePrepareResult.Deserialize(result);
-            Coverage coverage = new Coverage(coveragePrepareResultLoaded, logger.Object, _processWideContainer.GetService<IInstrumentationHelper>(), new FileSystem());
+            Coverage coverage = new Coverage(coveragePrepareResultLoaded, logger.Object, _processWideContainer.GetService<IInstrumentationHelper>(), new FileSystem(), new SourceRootTranslator(new Mock<ILogger>().Object, new FileSystem()));
             return coverage.GetCoverageResult();
         }
 

test/coverlet.core.tests/Helpers/SourceRootTranslatorTests.cs

@@ -28,6 +28,23 @@ public void Translate_Success()
             Assert.Equal(@"C:\git\coverlet\src\coverlet.core\obj\Debug\netstandard2.0\coverlet.core.pdb", translator.ResolveFilePath(fileToTranslate));
         }
 
+        [ConditionalFact]
+        [SkipOnOS(OS.Linux, "Windows path format only")]
+        [SkipOnOS(OS.MacOS, "Windows path format only")]
+        public void TranslatePathRoot_Success()
+        {
+            Mock<ILogger> logger = new Mock<ILogger>();
+            Mock<IFileSystem> fileSystem = new Mock<IFileSystem>();
+            fileSystem.Setup(f => f.Exists(It.IsAny<string>())).Returns((string p) =>
+            {
+                if (p == "testLib.dll" || p == @"C:\git\coverlet\src\coverlet.core\obj\Debug\netstandard2.0\coverlet.core.pdb" || p == "CoverletSourceRootsMapping") return true;
+                return false;
+            });
+            fileSystem.Setup(f => f.ReadAllLines(It.IsAny<string>())).Returns(File.ReadAllLines(@"TestAssets/CoverletSourceRootsMappingTest"));
+            SourceRootTranslator translator = new SourceRootTranslator("testLib.dll", logger.Object, fileSystem.Object);
+            Assert.Equal(@"C:\git\coverlet\", translator.ResolvePathRoot("/_/")[0].OriginalPath);
+        }
+
         [Fact]
         public void Translate_EmptyFile()
         {

test/coverlet.integration.tests/BaseTest.cs

@@ -200,7 +200,7 @@ private protected void AddCoverletCollectosRef(string projectPath)
             xml.Save(csproj);
         }
 
-        private protected string AddCollectorRunsettingsFile(string projectPath, string includeFilter = "[coverletsamplelib.integration.template]*DeepThought")
+        private protected string AddCollectorRunsettingsFile(string projectPath, string includeFilter = "[coverletsamplelib.integration.template]*DeepThought", bool sourceLink = false)
         {
             string runSettings =
 $@"<?xml version=""1.0"" encoding=""utf-8"" ?>
@@ -211,6 +211,7 @@ private protected string AddCollectorRunsettingsFile(string projectPath, string
            <Configuration>
             <Format>json,cobertura</Format>
             <Include>{includeFilter}</Include>
+            <UseSourceLink>{(sourceLink ? "true" : "false")}</UseSourceLink>
             <!-- We need to include test assembly because test and code to cover are in same template project -->
             <IncludeTestAssembly>true</IncludeTestAssembly>
         </Configuration>

test/coverlet.integration.tests/DeterministicBuild.cs

@@ -66,7 +66,7 @@ public void Msbuild()
             string sourceRootMappingFilePath = Path.Combine(_testProjectPath, "bin", _buildConfiguration, _testProjectTfm!, "CoverletSourceRootsMapping");
             Assert.True(File.Exists(sourceRootMappingFilePath), sourceRootMappingFilePath);
             Assert.True(!string.IsNullOrEmpty(File.ReadAllText(sourceRootMappingFilePath)), "Empty CoverletSourceRootsMapping file");
-            Assert.Equal(2, File.ReadAllLines(sourceRootMappingFilePath).Length);
+            Assert.Contains("=/_/", File.ReadAllText(sourceRootMappingFilePath));
 
             DotnetCli($"test -c {_buildConfiguration} --no-build /p:CollectCoverage=true /p:Include=\"[coverletsample.integration.determisticbuild]*DeepThought\" /p:IncludeTestAssembly=true", out standardOutput, out _, _testProjectPath);
             Assert.Contains("Test Run Successful.", standardOutput);
@@ -80,6 +80,30 @@ public void Msbuild()
             RunCommand("git", "clean -fdx", out _, out _, _testProjectPath);
         }
 
+        [Fact]
+        public void Msbuild_SourceLink()
+        {
+            CreateDeterministicTestPropsFile();
+            DotnetCli($"build -c {_buildConfiguration} /p:DeterministicSourcePaths=true", out string standardOutput, out string _, _testProjectPath);
+            Assert.Contains("Build succeeded.", standardOutput);
+            string sourceRootMappingFilePath = Path.Combine(_testProjectPath, "bin", _buildConfiguration, _testProjectTfm!, "CoverletSourceRootsMapping");
+            Assert.True(File.Exists(sourceRootMappingFilePath), sourceRootMappingFilePath);
+            Assert.True(!string.IsNullOrEmpty(File.ReadAllText(sourceRootMappingFilePath)), "Empty CoverletSourceRootsMapping file");
+            Assert.Contains("=/_/", File.ReadAllText(sourceRootMappingFilePath));
+
+            DotnetCli($"test -c {_buildConfiguration} --no-build /p:CollectCoverage=true /p:UseSourceLink=true /p:Include=\"[coverletsample.integration.determisticbuild]*DeepThought\" /p:IncludeTestAssembly=true", out standardOutput, out _, _testProjectPath);
+            Assert.Contains("Test Run Successful.", standardOutput);
+            Assert.Contains("| coverletsample.integration.determisticbuild | 100% | 100%   | 100%   |", standardOutput);
+            Assert.True(File.Exists(Path.Combine(_testProjectPath, "coverage.json")));
+            Assert.Contains("raw.githubusercontent.com", File.ReadAllText(Path.Combine(_testProjectPath, "coverage.json")));
+            AssertCoverage(standardOutput);
+
+            // Process exits hang on clean seem that process doesn't close, maybe some mbuild node reuse? btw manually tested
+            // DotnetCli("clean", out standardOutput, out standardError, _fixture.TestProjectPath);
+            // Assert.False(File.Exists(sourceRootMappingFilePath));
+            RunCommand("git", "clean -fdx", out _, out _, _testProjectPath);
+        }
+
         [Fact]
         public void Collectors()
         {
@@ -89,7 +113,7 @@ public void Collectors()
             string sourceRootMappingFilePath = Path.Combine(_testProjectPath, "bin", GetAssemblyBuildConfiguration().ToString(), _testProjectTfm!, "CoverletSourceRootsMapping");
             Assert.True(File.Exists(sourceRootMappingFilePath), sourceRootMappingFilePath);
             Assert.NotEmpty(File.ReadAllText(sourceRootMappingFilePath));
-            Assert.Equal(2, File.ReadAllLines(sourceRootMappingFilePath).Length);
+            Assert.Contains("=/_/", File.ReadAllText(sourceRootMappingFilePath));
 
             string runSettingsPath = AddCollectorRunsettingsFile(_testProjectPath, "[coverletsample.integration.determisticbuild]*DeepThought");
             Assert.True(DotnetCli($"test -c {_buildConfiguration} --no-build \"{_testProjectPath}\" --collect:\"XPlat Code Coverage\" --settings \"{runSettingsPath}\" --diag:{Path.Combine(_testProjectPath, "log.txt")}", out standardOutput, out _), standardOutput);
@@ -108,6 +132,35 @@ public void Collectors()
             RunCommand("git", "clean -fdx", out _, out _, _testProjectPath);
         }
 
+        [Fact]
+        public void Collectors_SourceLink()
+        {
+            CreateDeterministicTestPropsFile();
+            DotnetCli($"build -c {_buildConfiguration} /p:DeterministicSourcePaths=true", out string standardOutput, out string _, _testProjectPath);
+            Assert.Contains("Build succeeded.", standardOutput);
+            string sourceRootMappingFilePath = Path.Combine(_testProjectPath, "bin", GetAssemblyBuildConfiguration().ToString(), _testProjectTfm!, "CoverletSourceRootsMapping");
+            Assert.True(File.Exists(sourceRootMappingFilePath), sourceRootMappingFilePath);
+            Assert.NotEmpty(File.ReadAllText(sourceRootMappingFilePath));
+            Assert.Contains("=/_/", File.ReadAllText(sourceRootMappingFilePath));
+
+            string runSettingsPath = AddCollectorRunsettingsFile(_testProjectPath, "[coverletsample.integration.determisticbuild]*DeepThought", sourceLink: true);
+            Assert.True(DotnetCli($"test -c {_buildConfiguration} --no-build \"{_testProjectPath}\" --collect:\"XPlat Code Coverage\" --settings \"{runSettingsPath}\" --diag:{Path.Combine(_testProjectPath, "log.txt")}", out standardOutput, out _), standardOutput);
+            Assert.Contains("Test Run Successful.", standardOutput);
+            AssertCoverage(standardOutput);
+            Assert.Contains("raw.githubusercontent.com", File.ReadAllText(Directory.GetFiles(_testProjectPath, "coverage.cobertura.xml", SearchOption.AllDirectories).Single()));
+
+            // Check out/in process collectors injection
+            string dataCollectorLogContent = File.ReadAllText(Directory.GetFiles(_testProjectPath, "log.datacollector.*.txt").Single());
+            Assert.Contains("[coverlet]Initializing CoverletCoverageDataCollector with configuration:", dataCollectorLogContent);
+            Assert.Contains("[coverlet]Initialize CoverletInProcDataCollector", File.ReadAllText(Directory.GetFiles(_testProjectPath, "log.host.*.txt").Single()));
+            Assert.Contains("[coverlet]Mapping resolved", dataCollectorLogContent);
+
+            // Process exits hang on clean seem that process doesn't close, maybe some mbuild node reuse? btw manually tested
+            // DotnetCli("clean", out standardOutput, out standardError, _fixture.TestProjectPath);
+            // Assert.False(File.Exists(sourceRootMappingFilePath));
+            RunCommand("git", "clean -fdx", out _, out _, _testProjectPath);
+        }
+
         public void Dispose()
         {
             File.Delete(Path.Combine(_testProjectPath, PropsFileName));