Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
2 changes: 1 addition & 1 deletion src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ public CoverageResult GetCoverageResult()
}
}

var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results };
var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results, IsSourceLinkUsed = _useSourceLink };

if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && _fileSystem.Exists(_mergeWith))
{
Expand Down
1 change: 1 addition & 0 deletions src/coverlet.core/CoverageResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class CoverageResult
{
public string Identifier;
public Modules Modules;
public bool IsSourceLinkUsed;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I prefer to maintain switch name UseSourceLink

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

internal List<InstrumenterResult> InstrumentedResults;

internal CoverageResult() { }
Expand Down
39 changes: 37 additions & 2 deletions src/coverlet.core/Reporters/CoberturaReporter.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

using System.Text;
using System.Xml.Linq;

Expand Down Expand Up @@ -30,7 +32,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");
sources.Add(new XElement("source", string.Empty));
var rootDirs = GetRootDirs(result.Modules, result.IsSourceLinkUsed).ToList();
rootDirs.ForEach(x => sources.Add(new XElement("source", x)));

XElement packages = new XElement("packages");
foreach (var module in result.Modules)
Expand All @@ -48,7 +51,7 @@ public string Report(CoverageResult result)
{
XElement @class = new XElement("class");
@class.Add(new XAttribute("name", cls.Key));
@class.Add(new XAttribute("filename", document.Key));
@class.Add(new XAttribute("filename", GetRelativePathFromBase(rootDirs, document.Key, result.IsSourceLinkUsed)));
@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 @@ -129,5 +132,37 @@ public string Report(CoverageResult result)

return Encoding.UTF8.GetString(stream.ToArray());
}

private static IEnumerable<string> GetRootDirs(Modules modules, bool isSourceLinkUsed)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not

if (isSourceLinkUsed)
{
	return new[] { string.Empty };
}
else
{
	return modules.Values.SelectMany(k => k.Keys).Select(Directory.GetDirectoryRoot).Distinct();
}

Does it work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I tested it. Done.

{
if (isSourceLinkUsed) return new[] { string.Empty };

List<string> sources = new List<string>();

foreach (var module in modules)
{

sources.AddRange(
module.Value.Select(d => Path.GetDirectoryName(d.Key)));
}

sources = sources.Distinct().ToList();
return sources.Select(Directory.GetDirectoryRoot).Distinct();
}

private static string GetRelativePathFromBase(IEnumerable<string> basePaths, string path, bool isSourceLinkUsed)
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Nov 21, 2019

Choose a reason for hiding this comment

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

rename rootPaths and useSourceLink pls

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

{
if (isSourceLinkUsed) return path;

string relativePath = path;
basePaths.ToList().ForEach(basePath =>
{
if (path.StartsWith(basePath))
{
relativePath = path.Substring(basePath.Length);
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli Nov 21, 2019

Choose a reason for hiding this comment

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

what do you think about

            if (isSourceLinkUsed)
            {
                return path;
            }

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

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

            return path;

it seems more logic to me...when we found one valid root we can return immediatly, if we don't found any valid root(unexpected) we return original path, so we avoid exception and we'll generate a file(better than nothing), I added also an assertion so on debug we'll catch possible problem.

Copy link
Collaborator Author

@daveMueller daveMueller Dec 5, 2019

Choose a reason for hiding this comment

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

Yes, I agree. Done.

}
});
return relativePath;
}
}
}
4 changes: 2 additions & 2 deletions test/coverlet.core.tests/Reporters/CoberturaReporterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public void TestReport()
classes.Add("Coverlet.Core.Reporters.Tests.CoberturaReporterTests", methods);

Documents documents = new Documents();
documents.Add("doc.cs", classes);
documents.Add("/doc.cs", classes);

result.Modules = new Modules();
result.Modules.Add("module", documents);
Expand Down Expand Up @@ -102,7 +102,7 @@ public void TestEnsureParseMethodStringCorrectly(
classes.Add("Google.Protobuf.Reflection.MessageDescriptor", methods);

Documents documents = new Documents();
documents.Add("doc.cs", classes);
documents.Add("/doc.cs", classes);

result.Modules = new Modules();
result.Modules.Add("module", documents);
Expand Down