From 1f08b5d9464f19dcd250e59f3dc29ee5790fac45 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 11 Jul 2020 12:30:11 +0200 Subject: [PATCH 1/4] fix deterministic build with sourceLink --- .../Abstractions/ISourceRootTranslator.cs | 6 +- src/coverlet.core/Coverage.cs | 50 ++++++++++------- .../Helpers/SourceRootTranslator.cs | 13 +++-- .../CoverageResultTask.cs | 2 +- .../Coverage/InstrumenterHelper.cs | 2 +- .../Helpers/SourceRootTranslatorTests.cs | 17 ++++++ test/coverlet.integration.tests/BaseTest.cs | 3 +- .../DeterministicBuild.cs | 55 ++++++++++++++++++- 8 files changed, 119 insertions(+), 29 deletions(-) diff --git a/src/coverlet.core/Abstractions/ISourceRootTranslator.cs b/src/coverlet.core/Abstractions/ISourceRootTranslator.cs index 2d7323b38..44099fea2 100644 --- a/src/coverlet.core/Abstractions/ISourceRootTranslator.cs +++ b/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); + List ResolvePathRoot(string pathRoot); } } diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index d0de3faaa..877c49d04 100644 --- a/src/coverlet.core/Coverage.cs +++ b/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(); } - 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,35 @@ private string GetSourceLinkUrl(Dictionary sourceLinkDocuments, string key = sourceLinkDocument.Key; if (Path.GetFileName(key) != "*") continue; - string directoryDocument = Path.GetDirectoryName(document); - string sourceLinkRoot = Path.GetDirectoryName(key); - string relativePath = ""; + List rootMapping = _sourceRootTranslator.ResolvePathRoot(key.Substring(0, key.Length - 1)); + List rootPaths = rootMapping is null ? new List() { key } : new List(rootMapping.Select(m => m.OriginalPath)); - // if document is on repo root we skip relative path calculation - if (directoryDocument != sourceLinkRoot) + foreach (string keyMapping in rootPaths) { - 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; + } } } diff --git a/src/coverlet.core/Helpers/SourceRootTranslator.cs b/src/coverlet.core/Helpers/SourceRootTranslator.cs index 67cb9b085..01f096f7c 100644 --- a/src/coverlet.core/Helpers/SourceRootTranslator.cs +++ b/src/coverlet.core/Helpers/SourceRootTranslator.cs @@ -19,7 +19,7 @@ internal class SourceRootTranslator : ISourceRootTranslator private readonly IFileSystem _fileSystem; private readonly Dictionary> _sourceRootMapping; private const string MappingFileName = "CoverletSourceRootsMapping"; - private Dictionary _resolutionCache; + private Dictionary _resolutionCacheFiles; public SourceRootTranslator(ILogger logger, IFileSystem fileSystem) { @@ -76,11 +76,16 @@ private Dictionary> LoadSourceRootMapping(string return mapping; } + public List ResolvePathRoot(string pathRoot) + { + return _sourceRootMapping.TryGetValue(pathRoot, out List sourceRootMapping) ? sourceRootMapping : 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> 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()).Add(originalFileName, pathToCheck); + (_resolutionCacheFiles ??= new Dictionary()).Add(originalFileName, pathToCheck); _logger.LogVerbose($"Mapping resolved: '{originalFileName}' -> '{pathToCheck}'"); return pathToCheck; } diff --git a/src/coverlet.msbuild.tasks/CoverageResultTask.cs b/src/coverlet.msbuild.tasks/CoverageResultTask.cs index 792859862..12a807ea5 100644 --- a/src/coverlet.msbuild.tasks/CoverageResultTask.cs +++ b/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(), fileSystem); + coverage = new Coverage(CoveragePrepareResult.Deserialize(instrumenterStateStream), this._logger, ServiceProvider.GetService(), fileSystem, ServiceProvider.GetService()); } try diff --git a/test/coverlet.core.tests/Coverage/InstrumenterHelper.cs b/test/coverlet.core.tests/Coverage/InstrumenterHelper.cs index 951915c8d..2faabf3d7 100644 --- a/test/coverlet.core.tests/Coverage/InstrumenterHelper.cs +++ b/test/coverlet.core.tests/Coverage/InstrumenterHelper.cs @@ -63,7 +63,7 @@ public static CoverageResult GetCoverageResult(string filePath) }); _processWideContainer.GetRequiredService().SetLogger(logger.Object); CoveragePrepareResult coveragePrepareResultLoaded = CoveragePrepareResult.Deserialize(result); - Coverage coverage = new Coverage(coveragePrepareResultLoaded, logger.Object, _processWideContainer.GetService(), new FileSystem()); + Coverage coverage = new Coverage(coveragePrepareResultLoaded, logger.Object, _processWideContainer.GetService(), new FileSystem(), new SourceRootTranslator(new Mock().Object, new FileSystem())); return coverage.GetCoverageResult(); } diff --git a/test/coverlet.core.tests/Helpers/SourceRootTranslatorTests.cs b/test/coverlet.core.tests/Helpers/SourceRootTranslatorTests.cs index 4d5d360f7..590f1a233 100644 --- a/test/coverlet.core.tests/Helpers/SourceRootTranslatorTests.cs +++ b/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 logger = new Mock(); + Mock fileSystem = new Mock(); + fileSystem.Setup(f => f.Exists(It.IsAny())).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())).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() { diff --git a/test/coverlet.integration.tests/BaseTest.cs b/test/coverlet.integration.tests/BaseTest.cs index 5d7b3717c..5933bcb0a 100644 --- a/test/coverlet.integration.tests/BaseTest.cs +++ b/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 = $@" @@ -211,6 +211,7 @@ private protected string AddCollectorRunsettingsFile(string projectPath, string json,cobertura {includeFilter} + {(sourceLink ? "true" : "false")} true diff --git a/test/coverlet.integration.tests/DeterministicBuild.cs b/test/coverlet.integration.tests/DeterministicBuild.cs index 26880becd..75bce177a 100644 --- a/test/coverlet.integration.tests/DeterministicBuild.cs +++ b/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,29 @@ 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"))); + 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 +112,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 +131,34 @@ 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); + + // 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)); From f1dcde659000c6b77176f67cfe87aa90e3382d23 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 11 Jul 2020 12:48:52 +0200 Subject: [PATCH 2/4] add test assertions --- test/coverlet.integration.tests/DeterministicBuild.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/coverlet.integration.tests/DeterministicBuild.cs b/test/coverlet.integration.tests/DeterministicBuild.cs index 75bce177a..0dc6798ba 100644 --- a/test/coverlet.integration.tests/DeterministicBuild.cs +++ b/test/coverlet.integration.tests/DeterministicBuild.cs @@ -95,6 +95,7 @@ public void Msbuild_SourceLink() 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 @@ -146,6 +147,7 @@ public void Collectors_SourceLink() 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()); From 382f72060336be9a66e8241ed34cff03202b0fea Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 11 Jul 2020 12:52:23 +0200 Subject: [PATCH 3/4] update changelog --- Documentation/Changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index 02fa16bb8..ea1f4e432 100644 --- a/Documentation/Changelog.md +++ b/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 From b3487e8e9e9bcfc411422c95b65189ef15c6b8db Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 11 Jul 2020 12:57:41 +0200 Subject: [PATCH 4/4] cleanup --- src/coverlet.core/Abstractions/ISourceRootTranslator.cs | 2 +- src/coverlet.core/Coverage.cs | 6 ++---- src/coverlet.core/Helpers/SourceRootTranslator.cs | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/coverlet.core/Abstractions/ISourceRootTranslator.cs b/src/coverlet.core/Abstractions/ISourceRootTranslator.cs index 44099fea2..225e550b6 100644 --- a/src/coverlet.core/Abstractions/ISourceRootTranslator.cs +++ b/src/coverlet.core/Abstractions/ISourceRootTranslator.cs @@ -6,6 +6,6 @@ namespace Coverlet.Core.Abstractions internal interface ISourceRootTranslator { string ResolveFilePath(string originalFileName); - List ResolvePathRoot(string pathRoot); + IReadOnlyList ResolvePathRoot(string pathRoot); } } diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 877c49d04..ee1c2cacb 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -435,10 +435,8 @@ private string GetSourceLinkUrl(Dictionary sourceLinkDocuments, string key = sourceLinkDocument.Key; if (Path.GetFileName(key) != "*") continue; - List rootMapping = _sourceRootTranslator.ResolvePathRoot(key.Substring(0, key.Length - 1)); - List rootPaths = rootMapping is null ? new List() { key } : new List(rootMapping.Select(m => m.OriginalPath)); - - foreach (string keyMapping in rootPaths) + IReadOnlyList rootMapping = _sourceRootTranslator.ResolvePathRoot(key.Substring(0, key.Length - 1)); + foreach (string keyMapping in rootMapping is null ? new List() { key } : new List(rootMapping.Select(m => m.OriginalPath))) { string directoryDocument = Path.GetDirectoryName(document); string sourceLinkRoot = Path.GetDirectoryName(keyMapping); diff --git a/src/coverlet.core/Helpers/SourceRootTranslator.cs b/src/coverlet.core/Helpers/SourceRootTranslator.cs index 01f096f7c..9694a6a78 100644 --- a/src/coverlet.core/Helpers/SourceRootTranslator.cs +++ b/src/coverlet.core/Helpers/SourceRootTranslator.cs @@ -76,9 +76,9 @@ private Dictionary> LoadSourceRootMapping(string return mapping; } - public List ResolvePathRoot(string pathRoot) + public IReadOnlyList ResolvePathRoot(string pathRoot) { - return _sourceRootMapping.TryGetValue(pathRoot, out List sourceRootMapping) ? sourceRootMapping : null; + return _sourceRootMapping.TryGetValue(pathRoot, out List sourceRootMapping) ? sourceRootMapping.AsReadOnly() : null; } public string ResolveFilePath(string originalFileName)