ryanluker/vscode-coverage-gutters

MATLAB coverage.xml with abstract classes cannot be read

Closed this issue · 5 comments

fschwaiger commented

Summary

Upgrade cobertura to use a none archived library

Work

Original Ticket

Describe the bug
MATLAB has an option to generate code coverage reports in standard XML format. However, for abstract interface classes, there can be blocks that have no testable lines of code. The report generator will then produce empty <lines/> nodes:

<method branch-rate="NaN" line-rate="NaN" name="rnd" signature="sample = rnd(self, mu)">
  <lines/>
</method>

This behaviour is not anticipated by this extension. Following the stack trace (sorry, I don't have it at hand currently) leads to the following code snippet in this extension's JS, where the existence of c.methods[ 0 ].method[ x ].lines[ 0 ].line[ 0 ] is assumed:

                details: !c.methods || !c.methods[ 0 ].method ? [] : c.methods[ 0 ].method.map( function ( m )
                {
                    return {
                        name: m.$.name,
                        line: Number( m.lines[ 0 ].line[ 0 ].$.number ),
                        hit: Number( m.lines[ 0 ].line[ 0 ].$.hits )
                    };
                } )

To Reproduce
Steps to reproduce the behaviour:

  1. start with a valid coverage.xml
  2. empty a <lines/> node
  3. reload

Expected behaviour
The file should be loadable. I have not yet figured out what the correct value for missing lines would be. We could either check right before extracting the line number and return 0 or NAN:

line: (m.lines && m.lines[0].line) ? Number( m.lines[ 0 ].line[ 0 ].$.number ) : 0,

or filter out methods that have no lines in a preliminary filter() step:

c.methods[ 0 ].method.filter(...).map(...)

Desktop (please complete the following information):

  • OS: macOS
  • Extension Version: v2.7.2
  • VSCode Version: v1.55.2
fschwaiger commented

I can do a pull request for either of the two solutions named above. I would like to discuss though what you consider the best option before starting.

ryanluker commented

@fschwaiger Thanks for creating a ticket for this issue!
The code you are referencing is definitely not apart of this repo (coverage parsers provide a distinct typescript data structure that we consume). https://github.com/ryanluker/vscode-coverage-gutters/search?q=hit%3A+Number

Mostly likely you will need to submit a fix to one of the many parsers we support (some sadly are not very widely supported or maintained). https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/files/coverageparser.ts#L37-L46

Another option we have planned is to bring more of this parsing "in-house" by having the extension also do that work. #224 (comment)

Supported LCOV parsers
https://github.com/coverage-report/clover-json
https://github.com/vokal/cobertura-parse
https://github.com/vokal/jacoco-parse
https://github.com/davglass/lcov-parse

fschwaiger commented

I see, thank you for making me aware that this is within a dependency, I should have considered that. It definitely makes more sense to fix it there: https://github.com/vokal/cobertura-parse/blob/master/source/index.js

I will create an issue there and update here when done.

fschwaiger commented

@ryanluker I just saw that the dependency https://github.com/vokal/cobertura-parse is archived and quite outdated.

However it appear the following fork has attempted a fix: qingguo-yu/cobertura-parse@4cb916d

ryanluker commented

@fschwaiger Great find! I would swap out the version we have installed here with the forked one to see if the build passes and the issue is fixed 🤔 .