Skip to content

Relative paths lcov #1004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Relative paths lcov #1004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d4eb476
Refactored functions to determine base paths from CoberturaReporter i…
phaniva Oct 19, 2020
0e97f5d
Added dependency on IFilePathHelper for ReporterFactory (#263)
phaniva Oct 19, 2020
6454217
Updated LcovReporter to include only relative paths for source files …
phaniva Oct 19, 2020
35afa29
Merge branch 'master' of https://github.com/coverlet-coverage/coverle…
phaniva Nov 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/coverlet.collector/DataCollection/CoverageManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ internal class CoverageManager
public IReporter[] Reporters { get; }

public CoverageManager(CoverletSettings settings, TestPlatformEqtTrace eqtTrace, TestPlatformLogger logger, ICoverageWrapper coverageWrapper,
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper)
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper,
IFilePathHelper filePathHelper)
: this(settings,
settings.ReportFormats.Select(format =>
{
var reporterFactory = new ReporterFactory(format);
var reporterFactory = new ReporterFactory(format, filePathHelper);
if (!reporterFactory.IsValidFormat())
{
eqtTrace.Warning($"Invalid report format '{format}'");
Expand Down
4 changes: 3 additions & 1 deletion src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ private void OnSessionStart(object sender, SessionStartEventArgs sessionStartEve
// Get coverage and attachment managers
_coverageManager = new CoverageManager(coverletSettings, _eqtTrace, _logger, _coverageWrapper,
_serviceProvider.GetRequiredService<IInstrumentationHelper>(), _serviceProvider.GetRequiredService<IFileSystem>(),
_serviceProvider.GetRequiredService<ISourceRootTranslator>(), _serviceProvider.GetRequiredService<ICecilSymbolHelper>());
_serviceProvider.GetRequiredService<ISourceRootTranslator>(), _serviceProvider.GetRequiredService<ICecilSymbolHelper>(),
_serviceProvider.GetRequiredService<IFilePathHelper>());

// Instrument modules
_coverageManager.InstrumentModules();
Expand Down Expand Up @@ -223,6 +224,7 @@ private static IServiceCollection GetDefaultServiceCollection(TestPlatformEqtTra
serviceCollection.AddTransient<IRetryHelper, RetryHelper>();
serviceCollection.AddTransient<IProcessExitHandler, ProcessExitHandler>();
serviceCollection.AddTransient<IFileSystem, FileSystem>();
serviceCollection.AddTransient<IFilePathHelper, FilePathHelper>();
serviceCollection.AddTransient<ILogger, CoverletLogger>(_ => new CoverletLogger(eqtTrace, logger));
// We need to keep singleton/static semantics
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();
Expand Down
3 changes: 2 additions & 1 deletion src/coverlet.console/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ static int Main(string[] args)
serviceCollection.AddTransient<IProcessExitHandler, ProcessExitHandler>();
serviceCollection.AddTransient<IFileSystem, FileSystem>();
serviceCollection.AddTransient<ILogger, ConsoleLogger>();
serviceCollection.AddTransient<IFilePathHelper, FilePathHelper>();
// We need to keep singleton/static semantics
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();
serviceCollection.AddSingleton<ISourceRootTranslator, SourceRootTranslator>(provider => new SourceRootTranslator(provider.GetRequiredService<ILogger>(), provider.GetRequiredService<IFileSystem>()));
Expand Down Expand Up @@ -148,7 +149,7 @@ static int Main(string[] args)

foreach (var format in (formats.HasValue() ? formats.Values : new List<string>(new string[] { "json" })))
{
var reporter = new ReporterFactory(format).CreateReporter();
var reporter = new ReporterFactory(format, serviceProvider.GetRequiredService<IFilePathHelper>()).CreateReporter();
if (reporter == null)
{
throw new Exception($"Specified output format '{format}' is not supported");
Expand Down
10 changes: 10 additions & 0 deletions src/coverlet.core/Abstractions/IFilePathHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System.Collections.Generic;

namespace Coverlet.Core.Abstractions
{
interface IFilePathHelper
{
IEnumerable<string> GetBasePaths(IEnumerable<string> paths, bool useSourceLink);
string GetRelativePathFromBase(IEnumerable<string> basePaths, string path, bool useSourceLink);
}
}
93 changes: 93 additions & 0 deletions src/coverlet.core/Helpers/FilePathHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using Coverlet.Core.Abstractions;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;


namespace Coverlet.Core.Helpers
{
internal class FilePathHelper : IFilePathHelper
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Mar 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that extract a service is too much for a path helper...I mean we won't inject different implementation there are no different way to get that path from file system.
Should be enough add a new base class abstract ReporterBase and add this two method as protected and inherit the reporter that needs this logic.

Copy link
Author

@phaniva phaniva Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So only the reporter classes that need this behavior will inherit from ReporterBase and rest of them will continue to implement IReporter?
Why not inherit every reporter class from ReporterBase to keep things consistent?

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Mar 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I won't oppose, the important things is that we have less code as possible to maintain, let's go for base class inherited by reporters

Copy link
Author

@phaniva phaniva Mar 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. I am going to close this pr and open a new one with base class design.
Here is the new pr.
#1120

{
public IEnumerable<string> GetBasePaths(IEnumerable<string> paths, bool useSourceLink)
{
/*
Workflow

Path1 c:\dir1\dir2\file1.cs
Path2 c:\dir1\file2.cs
Path3 e:\dir1\file2.cs

1) Search for root dir
c:\ -> c:\dir1\dir2\file1.cs
c:\dir1\file2.cs
e:\ -> e:\dir1\file2.cs

2) Split path on directory separator i.e. for record c:\ ordered ascending by fragment elements
Path1 = [c:|dir1|file2.cs]
Path2 = [c:|dir1|dir2|file1.cs]

3) Find longest shared path comparing indexes
Path1[0] = Path2[0], ..., PathY[0] -> add to final fragment list
Path1[n] = Path2[n], ..., PathY[n] -> add to final fragment list
Path1[n+1] != Path2[n+1], ..., PathY[n+1] -> break, Path1[n] was last shared fragment

4) Concat created fragment list
*/
if (useSourceLink)
{
return new[] { string.Empty };
}

return paths.GroupBy(Directory.GetDirectoryRoot).Select(group =>
{
var splittedPaths = group.Select(absolutePath => absolutePath.Split(Path.DirectorySeparatorChar))
.OrderBy(absolutePath => absolutePath.Length).ToList();
if (splittedPaths.Count == 1)
{
return group.Key;
}

var basePathFragments = new List<string>();
bool stopSearch = false;
splittedPaths[0].Select((value, index) => (value, index)).ToList().ForEach(fragmentIndexPair =>
{
if (stopSearch)
{
return;
}

if (splittedPaths.All(sp => fragmentIndexPair.value.Equals(sp[fragmentIndexPair.index])))
{
basePathFragments.Add(fragmentIndexPair.value);
}
else
{
stopSearch = true;
}
});
return string.Concat(string.Join(Path.DirectorySeparatorChar.ToString(), basePathFragments), Path.DirectorySeparatorChar);
});
}

public string GetRelativePathFromBase(IEnumerable<string> basePaths, string path, bool useSourceLink)
{
if (useSourceLink)
{
return path;
}

foreach (var basePath in basePaths)
{
if (path.StartsWith(basePath))
{
return path.Substring(basePath.Length);
}
}

Debug.Assert(false, "Unexpected, we should find at least one path starts with one pre-build roots list");

return path;
}
}
}
93 changes: 11 additions & 82 deletions src/coverlet.core/Reporters/CoberturaReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Coverlet.Core.Abstractions;
using System;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -11,12 +12,19 @@ namespace Coverlet.Core.Reporters
{
internal class CoberturaReporter : IReporter
{
private IFilePathHelper _filePathHelper;

public ReporterOutputType OutputType => ReporterOutputType.File;

public string Format => "cobertura";

public string Extension => "cobertura.xml";

public CoberturaReporter(IFilePathHelper filePathHelper)
{
_filePathHelper = filePathHelper;
}

public string Report(CoverageResult result)
{
CoverageSummary summary = new CoverageSummary();
Expand All @@ -32,7 +40,8 @@ public string Report(CoverageResult result)
coverage.Add(new XAttribute("timestamp", (int)(DateTime.UtcNow - new DateTime(1970, 1, 1)).TotalSeconds));

XElement sources = new XElement("sources");
var absolutePaths = GetBasePaths(result.Modules, result.UseSourceLink).ToList();

var absolutePaths = _filePathHelper.GetBasePaths(result.Modules.Values.SelectMany(k => k.Keys), result.UseSourceLink).ToList();
absolutePaths.ForEach(x => sources.Add(new XElement("source", x)));

XElement packages = new XElement("packages");
Expand All @@ -51,7 +60,7 @@ public string Report(CoverageResult result)
{
XElement @class = new XElement("class");
@class.Add(new XAttribute("name", cls.Key));
@class.Add(new XAttribute("filename", GetRelativePathFromBase(absolutePaths, document.Key, result.UseSourceLink)));
@class.Add(new XAttribute("filename", _filePathHelper.GetRelativePathFromBase(absolutePaths, document.Key, result.UseSourceLink)));
@class.Add(new XAttribute("line-rate", (summary.CalculateLineCoverage(cls.Value).Percent / 100).ToString(CultureInfo.InvariantCulture)));
@class.Add(new XAttribute("branch-rate", (summary.CalculateBranchCoverage(cls.Value).Percent / 100).ToString(CultureInfo.InvariantCulture)));
@class.Add(new XAttribute("complexity", summary.CalculateCyclomaticComplexity(cls.Value)));
Expand Down Expand Up @@ -134,85 +143,5 @@ public string Report(CoverageResult result)
return Encoding.UTF8.GetString(stream.ToArray());
}

private static IEnumerable<string> GetBasePaths(Modules modules, bool useSourceLink)
{
/*
Workflow

Path1 c:\dir1\dir2\file1.cs
Path2 c:\dir1\file2.cs
Path3 e:\dir1\file2.cs

1) Search for root dir
c:\ -> c:\dir1\dir2\file1.cs
c:\dir1\file2.cs
e:\ -> e:\dir1\file2.cs

2) Split path on directory separator i.e. for record c:\ ordered ascending by fragment elements
Path1 = [c:|dir1|file2.cs]
Path2 = [c:|dir1|dir2|file1.cs]

3) Find longest shared path comparing indexes
Path1[0] = Path2[0], ..., PathY[0] -> add to final fragment list
Path1[n] = Path2[n], ..., PathY[n] -> add to final fragment list
Path1[n+1] != Path2[n+1], ..., PathY[n+1] -> break, Path1[n] was last shared fragment

4) Concat created fragment list
*/
if (useSourceLink)
{
return new[] { string.Empty };
}

return modules.Values.SelectMany(k => k.Keys).GroupBy(Directory.GetDirectoryRoot).Select(group =>
{
var splittedPaths = group.Select(absolutePath => absolutePath.Split(Path.DirectorySeparatorChar))
.OrderBy(absolutePath => absolutePath.Length).ToList();
if (splittedPaths.Count == 1)
{
return group.Key;
}

var basePathFragments = new List<string>();
bool stopSearch = false;
splittedPaths[0].Select((value, index) => (value, index)).ToList().ForEach(fragmentIndexPair =>
{
if (stopSearch)
{
return;
}

if (splittedPaths.All(sp => fragmentIndexPair.value.Equals(sp[fragmentIndexPair.index])))
{
basePathFragments.Add(fragmentIndexPair.value);
}
else
{
stopSearch = true;
}
});
return string.Concat(string.Join(Path.DirectorySeparatorChar.ToString(), basePathFragments), Path.DirectorySeparatorChar);
});
}

private static string GetRelativePathFromBase(IEnumerable<string> basePaths, string path, bool useSourceLink)
{
if (useSourceLink)
{
return path;
}

foreach (var basePath in basePaths)
{
if (path.StartsWith(basePath))
{
return path.Substring(basePath.Length);
}
}

Debug.Assert(false, "Unexpected, we should find at least one path starts with one pre-build roots list");

return path;
}
}
}
12 changes: 11 additions & 1 deletion src/coverlet.core/Reporters/LcovReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Coverlet.Core.Abstractions;
using System;
using System.Linq;
using System.Collections.Generic;
Expand All @@ -6,17 +7,26 @@ namespace Coverlet.Core.Reporters
{
internal class LcovReporter : IReporter
{
private IFilePathHelper _filePathHelper;

public ReporterOutputType OutputType => ReporterOutputType.File;

public string Format => "lcov";

public string Extension => "info";

public LcovReporter(IFilePathHelper filePathHelper)
{
_filePathHelper = filePathHelper;
}

public string Report(CoverageResult result)
{
CoverageSummary summary = new CoverageSummary();
List<string> lcov = new List<string>();

var absolutePaths = _filePathHelper.GetBasePaths(result.Modules.Values.SelectMany(k => k.Keys), result.UseSourceLink).ToList();

foreach (var module in result.Modules)
{
foreach (var doc in module.Value)
Expand All @@ -25,7 +35,7 @@ public string Report(CoverageResult result)
var docBranchCoverage = summary.CalculateBranchCoverage(doc.Value);
var docMethodCoverage = summary.CalculateMethodCoverage(doc.Value);

lcov.Add("SF:" + doc.Key);
lcov.Add("SF:" + _filePathHelper.GetRelativePathFromBase(absolutePaths, doc.Key, result.UseSourceLink));
foreach (var @class in doc.Value)
{
foreach (var method in @class.Value)
Expand Down
11 changes: 7 additions & 4 deletions src/coverlet.core/Reporters/ReporterFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Coverlet.Core.Abstractions;
using System;
using System.Linq;

Expand All @@ -7,13 +8,15 @@ internal class ReporterFactory
{
private string _format;
private IReporter[] _reporters;

public ReporterFactory(string format)
private IFilePathHelper _filePathHelper;

public ReporterFactory(string format, IFilePathHelper filePathHelper)
{
_format = format;
_filePathHelper = filePathHelper;
_reporters = new IReporter[] {
new JsonReporter(), new LcovReporter(),
new OpenCoverReporter(), new CoberturaReporter(),
new JsonReporter(), new LcovReporter(_filePathHelper),
new OpenCoverReporter(), new CoberturaReporter(_filePathHelper),
new TeamCityReporter()
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/coverlet.msbuild.tasks/CoverageResultTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public override bool Execute()
var coverageReportPaths = new List<ITaskItem>(formats.Length);
foreach (var format in formats)
{
var reporter = new ReporterFactory(format).CreateReporter();
var reporter = new ReporterFactory(format, ServiceProvider.GetRequiredService<IFilePathHelper>()).CreateReporter();
if (reporter == null)
{
throw new Exception($"Specified output format '{format}' is not supported");
Expand Down
1 change: 1 addition & 0 deletions src/coverlet.msbuild.tasks/InstrumentationTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public override bool Execute()
serviceCollection.AddTransient<IConsole, SystemConsole>();
serviceCollection.AddTransient<ILogger, MSBuildLogger>(_ => _logger);
serviceCollection.AddTransient<IRetryHelper, RetryHelper>();
serviceCollection.AddTransient<IFilePathHelper, FilePathHelper>();
// We cache resolutions
serviceCollection.AddSingleton<ISourceRootTranslator, SourceRootTranslator>(serviceProvider => new SourceRootTranslator(Path, serviceProvider.GetRequiredService<ILogger>(), serviceProvider.GetRequiredService<IFileSystem>()));
// We need to keep singleton/static semantics
Expand Down
Loading