Skip to content
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.

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

Reopen - Misleading coverage while using {get;set} #718

Open
tonyhallett opened this issue Dec 14, 2024 · 1 comment
Open

Reopen - Misleading coverage while using {get;set} #718

tonyhallett opened this issue Dec 14, 2024 · 1 comment

Comments

@tonyhallett
Copy link

As per #407 but not limited to the visual studio xml report. Similarly for cobertura.

Given an auto implemented property, if only one of the accessors is used they will both be shown as Covered and the corresponding CodeElement.CoverageQuota will be 100 for both.

This will also manifest itself when you pass in a cobertura file and get out a cobertura file.
Here is a portion of the in file

          <methods>
            <method line-rate="1" branch-rate="1" complexity="1" name="get_Throw" signature="()">
              <lines>
                <line number="20" hits="1" branch="False" />
              </lines>
            </method>
            <method line-rate="0" branch-rate="1" complexity="1" name="set_Throw" signature="(bool)">
              <lines>
                <line number="20" hits="0" branch="False" />
              </lines>
            </method>
          </methods>
          <lines>
            <line number="20" hits="1" branch="False" />

and the out file

          <methods>
            <method name="get_Throw" signature="()" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="20" hits="1" branch="false" />
              </lines>
            </method>
            <method name="set_Throw" signature="(bool)" line-rate="1" branch-rate="1" complexity="1">
              <lines>
                <line number="20" hits="1" branch="false" />
              </lines>
            </method>

          </methods>
          <lines>
            <line number="20" hits="1" branch="false" />

If the in file is an xml file then you can see that the data is available with the lines_covered attribute.

        <function id="8324" token="0x6000006" name="get_Throw()" namespace="TryCatchCoverageDemo" type_name="TryCatch" block_coverage="100.00" line_coverage="100.00" blocks_covered="1" blocks_not_covered="0" lines_covered="1" lines_partially_covered="0" lines_not_covered="0">
          <ranges>
            <range source_id="0" start_line="20" end_line="20" start_column="29" end_column="33" covered="yes" />
          </ranges>
        </function>
        <function id="8332" token="0x6000007" name="set_Throw(bool)" namespace="TryCatchCoverageDemo" type_name="TryCatch" block_coverage="0.00" line_coverage="0.00" blocks_covered="0" blocks_not_covered="1" lines_covered="0" lines_partially_covered="0" lines_not_covered="1">
          <ranges>
            <range source_id="0" start_line="20" end_line="20" start_column="34" end_column="38" covered="no" />
          </ranges>
        </function>

For the CoberturaParser https://github.com/danielpalme/ReportGenerator/blob/537c17c126cefe4da5de2bb885cee520cdf43b70/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L195C1-L262C10 the method line is ignored. The line will have hits 1. ( The CoverageElement.CoverageQuota is incorrect as is using the lineVisitStatus )

        private CodeFile ProcessFile(XElement[] classElements, Class @class, string className, string filePath)
        {
            var lines = classElements.Elements("lines")
                .Elements("line")
                .ToArray();

            var lineNumbers = lines
                .Select(l => l.Attribute("number").Value)
                .ToHashSet();

            var methodsOfFile = classElements
                .Elements("methods")
                .Elements("method")
                .ToArray();

            // not looking at the method line that states hits =0 *******************************
            var additionalLinesInMethodElement = methodsOfFile
                .Elements("lines")
                .Elements("line")
                .Where(l => !lineNumbers.Contains(l.Attribute("number").Value))
                .ToArray();

            var linesOfFile = lines.Concat(additionalLinesInMethodElement)
                .Select(line => new
                {
                    LineNumber = int.Parse(line.Attribute("number").Value, CultureInfo.InvariantCulture),
                    Visits = line.Attribute("hits").Value.ParseLargeInteger()
                })
                .OrderBy(seqpnt => seqpnt.LineNumber)
                .ToArray();

            var branches = GetBranches(lines);

            int[] coverage = new int[] { };
            LineVisitStatus[] lineVisitStatus = new LineVisitStatus[] { };

            if (linesOfFile.Length > 0)
            {
                coverage = new int[linesOfFile[linesOfFile.LongLength - 1].LineNumber + 1];
                lineVisitStatus = new LineVisitStatus[linesOfFile[linesOfFile.LongLength - 1].LineNumber + 1];

                for (int i = 0; i < coverage.Length; i++)
                {
                    coverage[i] = -1;
                }

                foreach (var line in linesOfFile)
                {
                    coverage[line.LineNumber] = line.Visits;

                    bool partiallyCovered = false;

                    if (branches.TryGetValue(line.LineNumber, out ICollection<Branch> branchesOfLine))
                    {
                        partiallyCovered = branchesOfLine.Any(b => b.BranchVisits == 0);
                    }

                    LineVisitStatus statusOfLine = line.Visits > 0 ? (partiallyCovered ? LineVisitStatus.PartiallyCovered : LineVisitStatus.Covered) : LineVisitStatus.NotCovered;
                    lineVisitStatus[line.LineNumber] = statusOfLine;
                }
            }

For the DynamicCodeCoverageParser
https://github.com/danielpalme/ReportGenerator/blob/537c17c126cefe4da5de2bb885cee520cdf43b70/src/ReportGenerator.Core/Parser/DynamicCodeCoverageParser.cs#L186C1-L245C10
Both the get and the set get to set the lineVisiitStatus so the line is covered and again the lineVisitStatus is used for the CoverageElement.CoverageQuota.

        private static CodeFile ProcessFile(XElement module, string fileId, ClassWithNamespace classWithNamespace, string filePath)
        {
            var methods = module
                .Elements("functions")
                .Elements("function")
                .Where(c => c.Attribute("namespace")?.Value == classWithNamespace.Namespace)
                .Where(c => c.Attribute("type_name").Value.Equals(classWithNamespace.ClassName, StringComparison.Ordinal)
                            || c.Attribute("type_name").Value.StartsWith(classWithNamespace.ClassName + ".", StringComparison.Ordinal))
                .Where(m => m.Elements("ranges").Elements("range").Any(r => r.Attribute("source_id").Value == fileId))
                .ToArray();

            var linesOfFile = methods
                .Elements("ranges")
                .Elements("range")
                .Where(l => l.Attribute("start_line").Value != "15732480")
                .Select(l => new
                {
                    LineNumberStart = int.Parse(l.Attribute("start_line").Value, CultureInfo.InvariantCulture),
                    LineNumberEnd = int.Parse(l.Attribute("end_line").Value, CultureInfo.InvariantCulture),

                   // correctly reading the attribute
                    Coverage = l.Attribute("covered").Value.Equals("no") ? 0 : 1,
                    Partial = l.Attribute("covered").Value.Equals("partial")
                })
                .OrderBy(seqpnt => seqpnt.LineNumberEnd)
                .ToArray();

            int[] coverage = new int[] { };
            LineVisitStatus[] lineVisitStatus = new LineVisitStatus[] { };

            if (linesOfFile.Length > 0)
            {
                coverage = new int[linesOfFile[linesOfFile.LongLength - 1].LineNumberEnd + 1];
                lineVisitStatus = new LineVisitStatus[linesOfFile[linesOfFile.LongLength - 1].LineNumberEnd + 1];

                for (int i = 0; i < coverage.Length; i++)
                {
                    coverage[i] = -1;
                }

                foreach (var seqpnt in linesOfFile)
                {
                    for (int lineNumber = seqpnt.LineNumberStart; lineNumber <= seqpnt.LineNumberEnd; lineNumber++)
                    {
                        // Both the get and set can set
                        coverage[lineNumber] = coverage[lineNumber] == -1 ? seqpnt.Coverage : Math.Min(coverage[lineNumber] + seqpnt.Coverage, 1);

                        // Both the get and set get to mark the LineVisitStatus as Covered
                        if (lineVisitStatus[lineNumber] != LineVisitStatus.Covered)
                        {
                            LineVisitStatus statusOfLine = seqpnt.Partial ? LineVisitStatus.PartiallyCovered : (seqpnt.Coverage == 1 ? LineVisitStatus.Covered : LineVisitStatus.NotCovered);
                            lineVisitStatus[lineNumber] = (LineVisitStatus)Math.Max((int)lineVisitStatus[lineNumber], (int)statusOfLine);
                        }
                    }
                }
            }

            var codeFile = new CodeFile(filePath, coverage, lineVisitStatus);

            SetMethodMetrics(codeFile, methods);
            SetCodeElements(codeFile, methods);

            return codeFile;
        }

Given that the information from the xml files are lost in the ParserResult and the FileAnalysis is solely line based then every builder will be rendering incorrectly.

For instance the CoberturaReportBuilder https://github.com/danielpalme/ReportGenerator/blob/537c17c126cefe4da5de2bb885cee520cdf43b70/src/ReportGenerator.Core/Reporting/Builders/CoberturaReportBuilder.cs#L52C21-L52C38 use the analysis lines for the range of a CodeElement and so get/set will have the same line-rate="1" and line hits="1"

        public void CreateClassReport(Class @class, IEnumerable<FileAnalysis> fileAnalyses)
        {
.....

            var classesElement = packageElement.Element("classes");

            foreach (var fileAnalysis in fileAnalyses)
            {
                decimal? fileComplexity = null;
                var classElement = new XElement(
                    "class",
                    new XAttribute("name", @class.Name),
                    new XAttribute("filename", fileAnalysis.Path));

                var methodsElement = new XElement("methods");

                foreach (var file in @class.Files)
                {
                    if (file.Path == fileAnalysis.Path)
                    {
                        foreach (var codeElement in file.CodeElements)
                        {
                            int index = codeElement.Name.LastIndexOf('(');

                            var methodLinesElement = new XElement("lines");

                            var methodElement = new XElement(
                                "method",
                                new XAttribute("name", index == -1 ? codeElement.Name : codeElement.Name.Substring(0, index)),
                                new XAttribute("signature", index == -1 ? string.Empty : codeElement.Name.Substring(index)),
                                methodLinesElement);

                           // uses the LineAnalysis for the range of the CodeElement
                            this.AddLineElements(
                                methodLinesElement,
                                fileAnalysis.Lines.Skip(codeElement.FirstLine - 1).Take(codeElement.LastLine - codeElement.FirstLine + 1),
                                out double methodLineRate,
                                out double methodBranchRate);

                            methodElement.Add(new XAttribute("line-rate", methodLineRate.ToString(CultureInfo.InvariantCulture)));
                            methodElement.Add(new XAttribute("branch-rate", methodBranchRate.ToString(CultureInfo.InvariantCulture)));
.......

                            methodsElement.Add(methodElement);
                        }

                        break;
                    }
                }
        private void AddLineElements(XElement parent, IEnumerable<LineAnalysis> lines, out double lineRate, out double branchRate)
        {
            int coveredLines = 0;
            int totalLines = 0;

            int coveredBranches = 0;
            int totalBranches = 0;

            foreach (var line in lines)
            {
                if (line.LineVisitStatus == LineVisitStatus.NotCoverable)
                {
                    continue;
                }

                totalLines++;

                if (line.LineVisits > 0)
                {
                    coveredLines++;
                }

                bool hasBranch = line.TotalBranches.GetValueOrDefault() > 0;

                var lineElement = new XElement(
                    "line",
                    new XAttribute("number", line.LineNumber),

                    // will be 1 for both get/set when only one is hit
                    new XAttribute("hits", line.LineVisits.ToString(CultureInfo.InvariantCulture)),
                    new XAttribute("branch", hasBranch ? "true" : "false"));

                if (hasBranch)
                {
                    int visitedBranchesLine = line.CoveredBranches.GetValueOrDefault();
                    int totalBranchesLine = line.TotalBranches.GetValueOrDefault();

                    coveredBranches += visitedBranchesLine;
                    totalBranches += totalBranchesLine;

                    double coverage = visitedBranchesLine / (double)totalBranchesLine;
                    lineElement.Add(new XAttribute(
                        "condition-coverage",
                        string.Format("{0}% ({1}/{2})", Math.Round(coverage * 100, MidpointRounding.AwayFromZero), visitedBranchesLine, totalBranchesLine)));
                }

                parent.Add(lineElement);
            }

            // will be the same for get and set
            lineRate = totalLines == 0 ? 1 : coveredLines / (double)totalLines;
            branchRate = totalBranches == 0 ? 1 : coveredBranches / (double)totalBranches;
        }
    }
@danielpalme
Copy link
Owner

I will have a look as soon as possible. It may take some time, since I'm pretty busy at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants