From 9dfe2e9ea2fe2bd535522d099a7246c5f9bd0462 Mon Sep 17 00:00:00 2001 From: Rik Crompton Date: Mon, 8 Jan 2024 11:04:01 +0000 Subject: [PATCH] Added a calculated figure for CRAP score for cobertura reports. closes #641 --- src/.gitignore | 3 +- .../Parser/CoberturaParserTest.cs | 118 ++++++++++++++---- .../Parser/CoberturaParser.cs | 75 +++++++---- 3 files changed, 147 insertions(+), 49 deletions(-) diff --git a/src/.gitignore b/src/.gitignore index cd80e8ac..8ed1f6a7 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -16,4 +16,5 @@ bin obj .DS_Store .AssemblyAttributes -node_modules \ No newline at end of file +node_modules +/_ReSharper.Caches \ No newline at end of file diff --git a/src/ReportGenerator.Core.Test/Parser/CoberturaParserTest.cs b/src/ReportGenerator.Core.Test/Parser/CoberturaParserTest.cs index 712ddea7..292af615 100644 --- a/src/ReportGenerator.Core.Test/Parser/CoberturaParserTest.cs +++ b/src/ReportGenerator.Core.Test/Parser/CoberturaParserTest.cs @@ -19,18 +19,17 @@ namespace Palmmedia.ReportGenerator.Core.Test.Parser [Collection("FileManager")] public class CoberturaParserTest { - private static readonly string FilePath1 = Path.Combine(FileManager.GetJavaReportDirectory(), "Cobertura2.1.1.xml"); + private static readonly string FilePathJavaReport = Path.Combine(FileManager.GetJavaReportDirectory(), "Cobertura2.1.1.xml"); + private static readonly string FilePathCSharpReport = Path.Combine(FileManager.GetCSharpReportDirectory(), "Cobertura_coverlet.xml"); - private readonly ParserResult parserResult; + private readonly ParserResult javaParserResult; + private readonly ParserResult csharpParserResult; public CoberturaParserTest() { - var filterMock = new Mock(); - filterMock.Setup(f => f.IsElementIncludedInReport(It.IsAny())).Returns(true); + this.javaParserResult = ParseReport(FilePathJavaReport); - var report = XDocument.Load(FilePath1); - new CoberturaReportPreprocessor().Execute(report); - this.parserResult = new CoberturaParser(filterMock.Object, filterMock.Object, filterMock.Object).Parse(report.Root); + this.csharpParserResult = ParseReport(FilePathCSharpReport); } /// @@ -39,7 +38,7 @@ public CoberturaParserTest() [Fact] public void SupportsBranchCoverage() { - Assert.True(this.parserResult.SupportsBranchCoverage); + Assert.True(this.javaParserResult.SupportsBranchCoverage); } /// @@ -48,7 +47,7 @@ public void SupportsBranchCoverage() [Fact] public void NumberOfLineVisitsTest() { - var fileAnalysis = GetFileAnalysis(this.parserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java"); + var fileAnalysis = GetFileAnalysis(this.javaParserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java"); Assert.Equal(1, fileAnalysis.Lines.Single(l => l.LineNumber == 15).LineVisits); Assert.Equal(1, fileAnalysis.Lines.Single(l => l.LineNumber == 17).LineVisits); Assert.Equal(0, fileAnalysis.Lines.Single(l => l.LineNumber == 20).LineVisits); @@ -61,7 +60,7 @@ public void NumberOfLineVisitsTest() [Fact] public void LineVisitStatusTest() { - var fileAnalysis = GetFileAnalysis(this.parserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java"); + var fileAnalysis = GetFileAnalysis(this.javaParserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java"); var line = fileAnalysis.Lines.Single(l => l.LineNumber == 1); Assert.Equal(LineVisitStatus.NotCoverable, line.LineVisitStatus); @@ -82,7 +81,7 @@ public void LineVisitStatusTest() [Fact] public void NumberOfFilesTest() { - Assert.Equal(7, this.parserResult.Assemblies.SelectMany(a => a.Classes).SelectMany(a => a.Files).Distinct().Count()); + Assert.Equal(7, this.javaParserResult.Assemblies.SelectMany(a => a.Classes).SelectMany(a => a.Files).Distinct().Count()); } /// @@ -91,8 +90,8 @@ public void NumberOfFilesTest() [Fact] public void FilesOfClassTest() { - Assert.Single(this.parserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.TestClass").Files); - Assert.Single(this.parserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.GenericClass").Files); + Assert.Single(this.javaParserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.TestClass").Files); + Assert.Single(this.javaParserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.GenericClass").Files); } /// @@ -101,7 +100,7 @@ public void FilesOfClassTest() [Fact] public void ClassesInAssemblyTest() { - Assert.Equal(7, this.parserResult.Assemblies.SelectMany(a => a.Classes).Count()); + Assert.Equal(7, this.javaParserResult.Assemblies.SelectMany(a => a.Classes).Count()); } /// @@ -110,7 +109,7 @@ public void ClassesInAssemblyTest() [Fact] public void AssembliesTest() { - Assert.Equal(2, this.parserResult.Assemblies.Count); + Assert.Equal(2, this.javaParserResult.Assemblies.Count); } /// @@ -119,7 +118,7 @@ public void AssembliesTest() [Fact] public void GetCoverableLinesOfClassTest() { - Assert.Equal(3, this.parserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.AbstractClass").CoverableLines); + Assert.Equal(3, this.javaParserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.AbstractClass").CoverableLines); } /// @@ -128,18 +127,73 @@ public void GetCoverableLinesOfClassTest() [Fact] public void MethodMetricsTest() { - var metrics = this.parserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.TestClass").Files.Single(f => f.Path == "C:\\temp\\test\\TestClass.java").MethodMetrics; + var metrics = this.javaParserResult.Assemblies.Single(a => a.Name == "test").Classes.Single(c => c.Name == "test.TestClass").Files.Single(f => f.Path == "C:\\temp\\test\\TestClass.java").MethodMetrics.ToArray(); Assert.Equal(4, metrics.Count()); - Assert.Equal("()V", metrics.First().FullName); - Assert.Equal(3, metrics.First().Metrics.Count()); - - Assert.Equal("Cyclomatic complexity", metrics.First().Metrics.ElementAt(0).Name); - Assert.Equal(0, metrics.First().Metrics.ElementAt(0).Value); - Assert.Equal("Line coverage", metrics.First().Metrics.ElementAt(1).Name); - Assert.Equal(100.0M, metrics.First().Metrics.ElementAt(1).Value); - Assert.Equal("Branch coverage", metrics.First().Metrics.ElementAt(2).Name); - Assert.Equal(100.0M, metrics.First().Metrics.ElementAt(2).Value); + + var initMethodMetric = metrics.First(); + Assert.Equal("()V", initMethodMetric.FullName); + Assert.Equal(4, initMethodMetric.Metrics.Count()); + + var complexityMetric = initMethodMetric.Metrics.Single(m => m.MetricType == MetricType.CodeQuality && m.Abbreviation == "cc"); + Assert.Equal("Cyclomatic complexity", complexityMetric.Name); + Assert.Equal(0, complexityMetric.Value); + + var lineCoverageMetric = initMethodMetric.Metrics.Single(m => m.MetricType == MetricType.CoveragePercentual && m.Abbreviation == "cov"); + Assert.Equal("Line coverage", lineCoverageMetric.Name); + Assert.Equal(100.0M, lineCoverageMetric.Value); + + var branchCoverageMetric = initMethodMetric.Metrics.Single(m => m.MetricType == MetricType.CoveragePercentual && m.Abbreviation == "bcov"); + Assert.Equal("Branch coverage", branchCoverageMetric.Name); + Assert.Equal(100.0M, branchCoverageMetric.Value); + + var crapScoreMetric = initMethodMetric.Metrics.Single(m => m.MetricType == MetricType.CodeQuality && m.Abbreviation == "crp"); + Assert.Equal("Crap Score", crapScoreMetric.Name); + Assert.Equal(0M, crapScoreMetric.Value); + } + + /// + /// A test for MethodMetrics + /// + [Theory] + [InlineData("Test", "Test.AbstractClass", "C:\\temp\\AbstractClass.cs", ".ctor()", 1, 1, 100, 100, 1)] + [InlineData("Test", "Test.AbstractClass_SampleImpl1", "C:\\temp\\AbstractClass.cs", "Method1()", 3, 1, 0, 100, 2)] + [InlineData("Test", "Test.PartialClass", "C:\\temp\\PartialClass.cs", "set_SomeProperty(System.Int32)", 4, 2, 66.66, 50, 2.15)] + [InlineData("Test", "Test.Program", "C:\\temp\\Program.cs", "Main(System.String[])", 4, 1, 89.65, 100, 1.00)] + [InlineData("Test", "Test.TestClass", "C:\\temp\\TestClass.cs", "SampleFunction()", 5, 4, 80, 50, 4.13)] + public void MethodMetricsTest_2(string assemblyName, string className, string filePath, string methodName, int expectedMethodMetrics, double expectedComplexity, double expectedLineCoverage, double expectedBranchCoverage, double expectedCrapScore) + { + var methodMetrics = csharpParserResult + .Assemblies.Single(a => a.Name == assemblyName) + .Classes.Single(c => c.Name == className) + .Files.Single(f => f.Path == filePath) + .MethodMetrics.ToArray(); + + Assert.Equal(expectedMethodMetrics, methodMetrics.Length); + + var methodMetric = methodMetrics.First(m => m.FullName == methodName); + Assert.Equal(methodName, methodMetric.FullName); + Assert.Equal(4, methodMetric.Metrics.Count()); + + var complexityMetric = methodMetric.Metrics.Single(m => m.MetricType == MetricType.CodeQuality && m.Abbreviation == "cc"); + Assert.Equal("Cyclomatic complexity", complexityMetric.Name); + Assert.True(complexityMetric.Value.HasValue); + Assert.Equal((decimal)expectedComplexity, complexityMetric.Value); + + var lineCoverageMetric = methodMetric.Metrics.Single(m => m.MetricType == MetricType.CoveragePercentual && m.Abbreviation == "cov"); + Assert.Equal("Line coverage", lineCoverageMetric.Name); + Assert.True(lineCoverageMetric.Value.HasValue); + Assert.Equal((decimal)expectedLineCoverage, lineCoverageMetric.Value); + + var branchCoverageMetric = methodMetric.Metrics.Single(m => m.MetricType == MetricType.CoveragePercentual && m.Abbreviation == "bcov"); + Assert.Equal("Branch coverage", branchCoverageMetric.Name); + Assert.True(branchCoverageMetric.Value.HasValue); + Assert.Equal((decimal)expectedBranchCoverage, branchCoverageMetric.Value); + + var crapScoreMetric = methodMetric.Metrics.Single(m => m.MetricType == MetricType.CodeQuality && m.Abbreviation == "crp"); + Assert.Equal("Crap Score", crapScoreMetric.Name); + Assert.True(crapScoreMetric.Value.HasValue); + Assert.Equal((decimal)expectedCrapScore, crapScoreMetric.Value); } /// @@ -148,7 +202,7 @@ public void MethodMetricsTest() [Fact] public void CodeElementsTest() { - var codeElements = GetFile(this.parserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java").CodeElements; + var codeElements = GetFile(this.javaParserResult.Assemblies, "test.TestClass", "C:\\temp\\test\\TestClass.java").CodeElements; Assert.Equal(4, codeElements.Count()); } @@ -162,5 +216,15 @@ public void CodeElementsTest() .Single(c => c.Name == className).Files .Single(f => f.Path == fileName) .AnalyzeFile(new CachingFileReader(new LocalFileReader(), 0, null)); + + private static ParserResult ParseReport(string filePath) + { + var filterMock = new Mock(); + filterMock.Setup(f => f.IsElementIncludedInReport(It.IsAny())).Returns(true); + + var report = XDocument.Load(filePath); + new CoberturaReportPreprocessor().Execute(report); + return new CoberturaParser(filterMock.Object, filterMock.Object, filterMock.Object).Parse(report.Root); + } } } diff --git a/src/ReportGenerator.Core/Parser/CoberturaParser.cs b/src/ReportGenerator.Core/Parser/CoberturaParser.cs index ceb943a7..ca88a112 100644 --- a/src/ReportGenerator.Core/Parser/CoberturaParser.cs +++ b/src/ReportGenerator.Core/Parser/CoberturaParser.cs @@ -324,46 +324,46 @@ private static void SetMethodMetrics(CodeFile codeFile, IEnumerable me var lineRate = method.Attribute("line-rate"); + decimal? coveragePercent = null; if (lineRate != null) { - decimal? value = null; + coveragePercent = GetCoberturaDecimalPercentageValue(lineRate.Value); - if (!"NaN".Equals(lineRate.Value, StringComparison.OrdinalIgnoreCase)) - { - value = Math.Round(100 * decimal.Parse(lineRate.Value.Replace(',', '.'), NumberStyles.Number | NumberStyles.AllowExponent, CultureInfo.InvariantCulture), 2, MidpointRounding.AwayFromZero); - } - - metrics.Add(Metric.Coverage(value)); + metrics.Add(Metric.Coverage(coveragePercent)); } var branchRate = method.Attribute("branch-rate"); if (branchRate != null) { - decimal? value = null; - - if (!"NaN".Equals(branchRate.Value, StringComparison.OrdinalIgnoreCase)) - { - value = Math.Round(100 * decimal.Parse(branchRate.Value.Replace(',', '.'), NumberStyles.Number | NumberStyles.AllowExponent, CultureInfo.InvariantCulture), 2, MidpointRounding.AwayFromZero); - } + decimal? value = GetCoberturaDecimalPercentageValue(branchRate.Value); metrics.Add(Metric.BranchCoverage(value)); } var cyclomaticComplexityAttribute = method.Attribute("complexity"); - + decimal? cyclomaticComplexity = null; if (cyclomaticComplexityAttribute != null) { - decimal? value = null; - - if (!"NaN".Equals(cyclomaticComplexityAttribute.Value, StringComparison.OrdinalIgnoreCase)) - { - value = Math.Round(decimal.Parse(cyclomaticComplexityAttribute.Value.Replace(',', '.'), NumberStyles.Number | NumberStyles.AllowExponent, CultureInfo.InvariantCulture), 2, MidpointRounding.AwayFromZero); - } + cyclomaticComplexity = GetCoberturaDecimalValue(cyclomaticComplexityAttribute.Value); metrics.Insert( 0, - Metric.CyclomaticComplexity(value)); + Metric.CyclomaticComplexity(cyclomaticComplexity)); + } + + if (cyclomaticComplexity.HasValue && coveragePercent.HasValue) + { + // https://testing.googleblog.com/2011/02/this-code-is-crap.html + // CRAP(m) = CC(m)^2 * U(m)^3 + CC(m) + // CC(m) <= Cyclomatic Complexity (e.g. 5) + // U(m) <= Uncovered percentage (e.g. 30% = 0.3) + var uncoveredPercent = (100f - (double)coveragePercent.Value) / 100.0; + var complexity = (double)cyclomaticComplexity.Value; + var crapScore = (Math.Pow(complexity, 2.0) * Math.Pow(uncoveredPercent, 3)) + complexity; + crapScore = Math.Round(crapScore, 2, MidpointRounding.AwayFromZero); + + metrics.Insert(0, Metric.CrapScore((decimal)crapScore)); } var methodMetric = new MethodMetric(fullName, shortName, metrics); @@ -382,6 +382,39 @@ private static void SetMethodMetrics(CodeFile codeFile, IEnumerable me } } + private static decimal? ParseCoberturaDecimalValue(string value) + { + decimal? result = null; + if (!"NaN".Equals(value, StringComparison.OrdinalIgnoreCase)) + { + result = decimal.Parse(value.Replace(',', '.'), NumberStyles.Number | NumberStyles.AllowExponent, CultureInfo.InvariantCulture); + } + + return result; + } + + private static decimal? GetCoberturaDecimalValue(string value) + { + decimal? result = ParseCoberturaDecimalValue(value); + if (result.HasValue) + { + result = Math.Round(result.Value, 2, MidpointRounding.AwayFromZero); + } + + return result; + } + + private static decimal? GetCoberturaDecimalPercentageValue(string value) + { + decimal? result = ParseCoberturaDecimalValue(value); + if (result.HasValue) + { + result = Math.Round(result.Value * 100, 2, MidpointRounding.AwayFromZero); + } + + return result; + } + /// /// Extracts the methods/properties of the given XElements. ///