Skip to content

Coverage percentage output value rounded down with max 2 decimal precision #397

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

Merged
merged 4 commits into from
May 7, 2019
Merged

Coverage percentage output value rounded down with max 2 decimal precision #397

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2bc7f0a
Coverage percentage output value is rounded down with max two decimal…
sasivishnu May 4, 2019
9586b91
Modifying Percent property in coverage details to return rounded down…
sasivishnu May 6, 2019
66dad3b
Wrongly called Line coverage instead of branch coverage in Cobertura …
sasivishnu May 6, 2019
2201898
When no line exists, Percent should return default percentage value 100.
sasivishnu May 6, 2019
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
12 changes: 6 additions & 6 deletions src/coverlet.console/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,15 @@ static int Main(string[] args)
var summary = new CoverageSummary();
int numModules = result.Modules.Count;

var totalLinePercent = summary.CalculateLineCoverage(result.Modules).Percent * 100;
var totalBranchPercent = summary.CalculateBranchCoverage(result.Modules).Percent * 100;
var totalMethodPercent = summary.CalculateMethodCoverage(result.Modules).Percent * 100;
var totalLinePercent = summary.CalculateLineCoverage(result.Modules).GetCoveragePercentage();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can just be var totalLinePercent = summary.CalculateLineCoverage(result.Modules).Percent if you update the Percent property to contain the necessary calculations. Applies everywhere this new method is used

var totalBranchPercent = summary.CalculateBranchCoverage(result.Modules).GetCoveragePercentage();
var totalMethodPercent = summary.CalculateMethodCoverage(result.Modules).GetCoveragePercentage();

foreach (var _module in result.Modules)
{
var linePercent = summary.CalculateLineCoverage(_module.Value).Percent * 100;
var branchPercent = summary.CalculateBranchCoverage(_module.Value).Percent * 100;
var methodPercent = summary.CalculateMethodCoverage(_module.Value).Percent * 100;
var linePercent = summary.CalculateLineCoverage(_module.Value).GetCoveragePercentage();
var branchPercent = summary.CalculateBranchCoverage(_module.Value).GetCoveragePercentage();
var methodPercent = summary.CalculateMethodCoverage(_module.Value).GetCoveragePercentage();

coverageTable.AddRow(Path.GetFileNameWithoutExtension(_module.Key), $"{linePercent}%", $"{branchPercent}%", $"{methodPercent}%");
}
Expand Down
13 changes: 12 additions & 1 deletion src/coverlet.core/CoverageDetails.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,18 @@ public class CoverageDetails
public int Total { get; internal set; }
public double Percent
{
get => Math.Round(Total == 0 ? 1 : Covered / Total, 3);
get => Math.Round(Total == 0 ? 1 : Covered / Total, 4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

All rounding down and decimal place code should go here

}

public double GetCoveragePercentage()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use property CoveragePercentace { get }

Copy link
Contributor Author

@sasivishnu sasivishnu May 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I can do that. Will convert method into prop CoveragePercentage { get }

{
double percentage = Percent * 100;
return RoundDown(percentage);
}

private double RoundDown(double percentage)
Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli May 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Can you avoid RoundDown helper and use inline directly on CoveragePercentace prop, if I'm not mistaken this helper is private and used only in one method so it's not so useful extract it as helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, its used only in one place as of now.

I forget to ask this earlier, Average Coverage Percentage value could go more than 2 decimal precision since its calculated inline.
coverageTable.AddRow("Average", $"{totalLinePercent / numModules}%", $"{totalBranchPercent / numModules}%", $"{totalMethodPercent / numModules}%");
Should I Floor the value with 2 decimal precision there also? Should I create MathHelper class under Helper folder, add this RoundDown method in the class & refer it in CoveragePercentage property and also during Average calculation? Is this fine?

Copy link
Contributor Author

@sasivishnu sasivishnu May 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@MarcoRossignoli I have done other changes and ready for check-in. Also before & after report formats output.

I am waiting to see your reply on what to do with "Average Percentage" calculation and then check-in code fully at once either way.

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli May 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I did deeper review...and I think that we should change a bit the idea.
We should avoid to change CoverageDetail object only to "round data", I don't like the idea to have members with same meaning but only different "formatting" seems a duplication to me, I mean GetCoveragePercentage is Percent formatted.
I like the idea of double MathHelper.RoundPercentage(double) in a file MathHelper.cs under Helpers folder and apply this helper in every place we need, in this way we can also apply it on Average Coverage Percentage.
For instance

var totalLinePercent =  MathHelper.RoundPercentage(summary.CalculateLineCoverage(result.Modules).Percent * 100);
...
var method = MathHelper.RoundPercentage(summary.CalculateMethodCoverage(module.Value));
...
 coverageTable.AddRow("Average", $"{MathHelper.RoundPercentage(totalLinePercent / numModules)}%",...

In this way we have also cetralized "percentage rounding" that we can apply in a uniform way in all codebase project.

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli May 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

could be cute to have an extension method

namespace Coverlet.Core.Helpers
{
   public static MathHelper 
   {
       public static double RoundPercentage(this double value)
       {
         ...
       }
   }
...

Copy link
Collaborator

Choose a reason for hiding this comment

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

The idea of helper was for future possible use of rounding percentage everywhere in same way, btw I'm ok also with this with "fix" where we need like above.

Copy link
Contributor Author

@sasivishnu sasivishnu May 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I am planning to do both. Use Percent property only but add extension method in new MathHelper.cs class as public static double RoundPercentage(this double value).

So, it will be like Percent => { Total == 0? 1: ((Covered/Total)*100).RoundPercentage() }

Copy link
Collaborator

@MarcoRossignoli MarcoRossignoli May 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@sasivishnu avoid helper at all, less code to review and maintain, if in future we'll need to reuse we'll move to helper, put all round alg in prop getter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @sasivishnu, let me know when you'd done this so I can merge. The rest of the code seems good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tonerdo Yep, I am done with my changes and pushed latest one.

{
return Math.Floor(percentage * 100) / 100;
}
}
}
18 changes: 9 additions & 9 deletions src/coverlet.core/CoverageResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summar
{
foreach (var module in Modules)
{
var line = summary.CalculateLineCoverage(module.Value).Percent * 100;
var branch = summary.CalculateBranchCoverage(module.Value).Percent * 100;
var method = summary.CalculateMethodCoverage(module.Value).Percent * 100;
var line = summary.CalculateLineCoverage(module.Value).GetCoveragePercentage();
var branch = summary.CalculateBranchCoverage(module.Value).GetCoveragePercentage();
var method = summary.CalculateMethodCoverage(module.Value).GetCoveragePercentage();

if ((thresholdTypes & ThresholdTypeFlags.Line) != ThresholdTypeFlags.None)
{
Expand Down Expand Up @@ -203,9 +203,9 @@ public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summar

foreach (var module in Modules)
{
line += summary.CalculateLineCoverage(module.Value).Percent * 100;
branch += summary.CalculateBranchCoverage(module.Value).Percent * 100;
method += summary.CalculateMethodCoverage(module.Value).Percent * 100;
line += summary.CalculateLineCoverage(module.Value).GetCoveragePercentage();
branch += summary.CalculateBranchCoverage(module.Value).GetCoveragePercentage();
method += summary.CalculateMethodCoverage(module.Value).GetCoveragePercentage();
}

if ((thresholdTypes & ThresholdTypeFlags.Line) != ThresholdTypeFlags.None)
Expand All @@ -229,9 +229,9 @@ public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summar
break;
case ThresholdStatistic.Total:
{
var line = summary.CalculateLineCoverage(Modules).Percent * 100;
var branch = summary.CalculateBranchCoverage(Modules).Percent * 100;
var method = summary.CalculateMethodCoverage(Modules).Percent * 100;
var line = summary.CalculateLineCoverage(Modules).GetCoveragePercentage();
var branch = summary.CalculateBranchCoverage(Modules).GetCoveragePercentage();
var method = summary.CalculateMethodCoverage(Modules).GetCoveragePercentage();

if ((thresholdTypes & ThresholdTypeFlags.Line) != ThresholdTypeFlags.None)
{
Expand Down
4 changes: 2 additions & 2 deletions src/coverlet.core/Reporters/CoberturaReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ public string Report(CoverageResult result)
{
var branches = meth.Value.Branches.Where(b => b.Line == ln.Key).ToList();
var branchInfoCoverage = summary.CalculateBranchCoverage(branches);
line.Add(new XAttribute("condition-coverage", $"{branchInfoCoverage.Percent * 100}% ({branchInfoCoverage.Covered}/{branchInfoCoverage.Total})"));
line.Add(new XAttribute("condition-coverage", $"{branchInfoCoverage.GetCoveragePercentage()}% ({branchInfoCoverage.Covered}/{branchInfoCoverage.Total})"));
XElement conditions = new XElement("conditions");
var byOffset = branches.GroupBy(b => b.Offset).ToDictionary(b => b.Key, b => b.ToList());
foreach (var entry in byOffset)
{
XElement condition = new XElement("condition");
condition.Add(new XAttribute("number", entry.Key));
condition.Add(new XAttribute("type", entry.Value.Count() > 2 ? "switch" : "jump")); // Just guessing here
condition.Add(new XAttribute("coverage", $"{summary.CalculateBranchCoverage(entry.Value).Percent * 100}%"));
condition.Add(new XAttribute("coverage", $"{summary.CalculateBranchCoverage(entry.Value).GetCoveragePercentage()}%"));
conditions.Add(condition);
}

Expand Down
16 changes: 8 additions & 8 deletions src/coverlet.core/Reporters/OpenCoverReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public string Report(CoverageResult result)

method.Add(new XAttribute("cyclomaticComplexity", methCyclomaticComplexity.ToString()));
method.Add(new XAttribute("nPathComplexity", "0"));
method.Add(new XAttribute("sequenceCoverage", Math.Round(methLineCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
method.Add(new XAttribute("branchCoverage", Math.Round(methBranchCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
method.Add(new XAttribute("sequenceCoverage", methLineCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
method.Add(new XAttribute("branchCoverage", methBranchCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
method.Add(new XAttribute("isConstructor", meth.Key.Contains("ctor").ToString()));
method.Add(new XAttribute("isGetter", meth.Key.Contains("get_").ToString()));
method.Add(new XAttribute("isSetter", meth.Key.Contains("set_").ToString()));
Expand Down Expand Up @@ -158,8 +158,8 @@ public string Report(CoverageResult result)
methodSummary.Add(new XAttribute("visitedSequencePoints", methLineCoverage.Covered.ToString()));
methodSummary.Add(new XAttribute("numBranchPoints", methBranchCoverage.Total.ToString()));
methodSummary.Add(new XAttribute("visitedBranchPoints", methBranchCoverage.Covered.ToString()));
methodSummary.Add(new XAttribute("sequenceCoverage", Math.Round(methLineCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
methodSummary.Add(new XAttribute("branchCoverage", Math.Round(methBranchCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
methodSummary.Add(new XAttribute("sequenceCoverage", methLineCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
methodSummary.Add(new XAttribute("branchCoverage", methBranchCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
methodSummary.Add(new XAttribute("maxCyclomaticComplexity", methCyclomaticComplexity.ToString()));
methodSummary.Add(new XAttribute("minCyclomaticComplexity", methCyclomaticComplexity.ToString()));
methodSummary.Add(new XAttribute("visitedClasses", "0"));
Expand Down Expand Up @@ -192,8 +192,8 @@ public string Report(CoverageResult result)
classSummary.Add(new XAttribute("visitedSequencePoints", classLineCoverage.Covered.ToString()));
classSummary.Add(new XAttribute("numBranchPoints", classBranchCoverage.Total.ToString()));
classSummary.Add(new XAttribute("visitedBranchPoints", classBranchCoverage.Covered.ToString()));
classSummary.Add(new XAttribute("sequenceCoverage", Math.Round(classLineCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
classSummary.Add(new XAttribute("branchCoverage", Math.Round(classBranchCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
classSummary.Add(new XAttribute("sequenceCoverage", classLineCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
classSummary.Add(new XAttribute("branchCoverage", classBranchCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
classSummary.Add(new XAttribute("maxCyclomaticComplexity", classMaxCyclomaticComplexity.ToString()));
classSummary.Add(new XAttribute("minCyclomaticComplexity", classMinCyclomaticComplexity.ToString()));
classSummary.Add(new XAttribute("visitedClasses", classVisited ? "1" : "0"));
Expand Down Expand Up @@ -223,8 +223,8 @@ public string Report(CoverageResult result)
coverageSummary.Add(new XAttribute("visitedSequencePoints", moduleLineCoverage.Covered.ToString()));
coverageSummary.Add(new XAttribute("numBranchPoints", moduleBranchCoverage.Total.ToString()));
coverageSummary.Add(new XAttribute("visitedBranchPoints", moduleBranchCoverage.Covered.ToString()));
coverageSummary.Add(new XAttribute("sequenceCoverage", Math.Round(moduleLineCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
coverageSummary.Add(new XAttribute("branchCoverage", Math.Round(moduleBranchCoverage.Percent * 100, 2).ToString("G", CultureInfo.InvariantCulture)));
coverageSummary.Add(new XAttribute("sequenceCoverage", moduleLineCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
coverageSummary.Add(new XAttribute("branchCoverage", moduleBranchCoverage.GetCoveragePercentage().ToString("G", CultureInfo.InvariantCulture)));
coverageSummary.Add(new XAttribute("maxCyclomaticComplexity", moduleMaxCyclomaticComplexity.ToString()));
coverageSummary.Add(new XAttribute("minCyclomaticComplexity", moduleMinCyclomaticComplexity.ToString()));
coverageSummary.Add(new XAttribute("visitedClasses", visitedClasses.ToString()));
Expand Down
6 changes: 3 additions & 3 deletions src/coverlet.core/Reporters/TeamCityReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public string Report(CoverageResult result)
private void OutputLineCoverage(CoverageDetails coverageDetails, StringBuilder builder)
{
// The total number of lines
OutputTeamCityServiceMessage("CodeCoverageL", coverageDetails.Percent * 100, builder);
OutputTeamCityServiceMessage("CodeCoverageL", coverageDetails.GetCoveragePercentage(), builder);

// The number of covered lines
OutputTeamCityServiceMessage("CodeCoverageAbsLCovered", coverageDetails.Covered, builder);
Expand All @@ -46,7 +46,7 @@ private void OutputLineCoverage(CoverageDetails coverageDetails, StringBuilder b
private void OutputBranchCoverage(CoverageDetails coverageDetails, StringBuilder builder)
{
// The total number of branches
OutputTeamCityServiceMessage("CodeCoverageR", coverageDetails.Percent * 100, builder);
OutputTeamCityServiceMessage("CodeCoverageR", coverageDetails.GetCoveragePercentage(), builder);

// The number of covered branches
OutputTeamCityServiceMessage("CodeCoverageAbsRCovered", coverageDetails.Covered, builder);
Expand All @@ -58,7 +58,7 @@ private void OutputBranchCoverage(CoverageDetails coverageDetails, StringBuilder
private void OutputMethodCoverage(CoverageDetails coverageDetails, StringBuilder builder)
{
// The total number of methods
OutputTeamCityServiceMessage("CodeCoverageM", coverageDetails.Percent * 100, builder);
OutputTeamCityServiceMessage("CodeCoverageM", coverageDetails.GetCoveragePercentage(), builder);

// The number of covered methods
OutputTeamCityServiceMessage("CodeCoverageAbsMCovered", coverageDetails.Covered, builder);
Expand Down
12 changes: 6 additions & 6 deletions src/coverlet.msbuild.tasks/CoverageResultTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,15 @@ public override bool Execute()
var summary = new CoverageSummary();
int numModules = result.Modules.Count;

var totalLinePercent = summary.CalculateLineCoverage(result.Modules).Percent * 100;
var totalBranchPercent = summary.CalculateBranchCoverage(result.Modules).Percent * 100;
var totalMethodPercent = summary.CalculateMethodCoverage(result.Modules).Percent * 100;
var totalLinePercent = summary.CalculateLineCoverage(result.Modules).GetCoveragePercentage();
var totalBranchPercent = summary.CalculateBranchCoverage(result.Modules).GetCoveragePercentage();
var totalMethodPercent = summary.CalculateMethodCoverage(result.Modules).GetCoveragePercentage();

foreach (var module in result.Modules)
{
var linePercent = summary.CalculateLineCoverage(module.Value).Percent * 100;
var branchPercent = summary.CalculateBranchCoverage(module.Value).Percent * 100;
var methodPercent = summary.CalculateMethodCoverage(module.Value).Percent * 100;
var linePercent = summary.CalculateLineCoverage(module.Value).GetCoveragePercentage();
var branchPercent = summary.CalculateBranchCoverage(module.Value).GetCoveragePercentage();
var methodPercent = summary.CalculateMethodCoverage(module.Value).GetCoveragePercentage();

coverageTable.AddRow(Path.GetFileNameWithoutExtension(module.Key), $"{linePercent}%", $"{branchPercent}%", $"{methodPercent}%");
}
Expand Down
Loading