byte-physics / igortest

Igor Pro Universal Testing Framework
https://docs.byte-physics.de/igor-unit-testing-framework/
BSD 3-Clause "New" or "Revised" License
7 stars 2 forks source link

Add Cobertura support #396

Closed Garados007 closed 1 year ago

Garados007 commented 1 year ago

Add support for coverage output in the Cobertura format.

Close #206

Garados007 commented 1 year ago

@t-b @MichaelHuth Ready for review.

I didn't get a view inside Gitlab but Gitlab is accepting the files and the output is correct according DTD. VSCode Coverage Gutter and Report Generator are accepting the output and show correct results with the Cobertura files.

There are some issues instrumenting igortest-tracing.ipf as its doing something weird in Igor Pro (some kind of Stack Overflow) so I have removed this file from coverage tests.

Garados007 commented 1 year ago

I have looked into the python, php, c++ and javascript implementation which are listed in the Gitlab documentation. I was able to use a lot of it and learn how some formulas work. In the end I could implement the larger schema 04 instead the discussed schema 03. There are only some things unresolved and set to a constant value in the output:

t-b commented 1 year ago

How does a condition relate to a for statement?

Garados007 commented 1 year ago

How does a condition relate to a for statement?

Just as for(start; condition; increment). Before each loop iteration the condition is checked if the loop body should be executed. You can also imagine a for-loop to be fancy syntax for an if-statement and a jump.

That was just something I stumbled across when looking what other libraries do. I don't think we have to add this for now.

t-b commented 1 year ago

Got it. Thanks for the explanation.

t-b commented 1 year ago

Could you try if https://github.com/aconrad/pycobertura/ can grok the files as well?

Garados007 commented 1 year ago

Could you try if https://github.com/aconrad/pycobertura/ can grok the files as well?

pycobertura could read the generated Cobertura files and highlighted the correct lines but there are some downfalls:

  1. pycobertura uses only one input file. Someone has to merge all cobertura files together or have to call pycobertura multiple times.
  2. pycobertura ignores the sources in the Cobertura specification and the user has to provide it using the CLI. This is only a limitation of pycobertura as other tools could use it without any problems.

But I could solve these issues after installing ReportGenerator to merge all Cobertura files into one and running this script:

#!/bin/bash
reportgenerator "-reports:**/Cobertura_*.xml" -targetdir:.report -reporttypes:Cobertura
pycobertura show --format html --output coverage2.html ".report/Cobertura.xml"
// remove full paths from HTML file
if [ ! -e "$MINGW_CHOST" ]; then
    filter="$(pwd | sed -r 's@^/([a-z])/@\U\1:\\\\@; s@/@\\\\@g')"
else
    filter="$(pwd)"
fi
sed -i "s@$filter@@g" coverage2.html

Attachments:

I have renamed the attachments to *.txt to circumvent upload filter.

Garados007 commented 1 year ago

While testing I found a minor bug which has been fixed right now.

t-b commented 1 year ago

Thanks, the pycobertura reports look good.

t-b commented 1 year ago

@Garados007 Do I see correctly that pycobertura does not support branch coverage?

Garados007 commented 1 year ago

Do I see correctly that pycobertura does not support branch coverage?

Yes. Also the HTML output doesn't show any coverage for empty lines (even if it's exists and counted). pycobertura is quite limited in some things.

t-b commented 1 year ago

@Garados007 Please ping us once this is ready for review.

Garados007 commented 1 year ago

@t-b @MichaelHuth Ready for review

t-b commented 1 year ago

First of all, very nice! I've played around with it and it works and produces nice cobertura files! And works with MIES Basic.pxp mostly ;)

I do have some comments though:

igortest-tracing-cobertura.ipf:

And I also tested it with Basics.pxp from MIES. It worked but could see a few improvements:

Cannot get the full function name of "WaveBuilder" in procedure "MIES_WaveBuilder_Macro.ipf": Function WaveBuilder in procedure file MIES_WaveBuilder_Macro.ipf is unknown

WaveBuilder is Macro, so there is no full function name. Maybe that error should just be handled silently?

Skip report generation of "MIES_Constants.ipf" as no procedure file found.

The english of this message is questionable, but I also fail to see what is wrong with that file. It does not have any functions/macros.

Can we also make the history output less verbose? I did like what we do for the HTML generation with just outputting one . for each file.

Skip report generation of "MIES_Include.ipf" as no procedure file found.

This file is not instrumented.

For the report generation with reportgenerator remember to undo Z_ before, so that the line numbers match.

I think functions with UTF_NOINSTRUMENTATION should not be uncovered but uninstrumented or? And #ifdef code which is not compiled should also not be treated as uncovered. See ASYNC_ThreadREadOut().

If both issues stem from instrumentation itself, please create an issue.

Basic.zip

t-b commented 1 year ago

Forgot to reassign ;)

t-b commented 1 year ago

@Garados007 In case you are wondering how to fix the reuse tool. All used licenses have to be stored in LICENSES. reuse download does that.

Garados007 commented 1 year ago

FilePathToXML: Use ParseFilePath with 5 instead of ReplaceString

ParseFilePath(5, winPath, "/", 0, 0) is not supported in Igor 9. I keep the current approach.

I think functions with UTF_NOINSTRUMENTATION should not be uncovered but uninstrumented or? And #ifdef code which is not compiled should also not be treated as uncovered. See ASYNC_ThreadREadOut().

I fixed it by including #414 and use that data instead.

In case you are wondering how to fix the reuse tool. All used licenses have to be stored in LICENSES. reuse download does that.

Thanks. I have looked for it manually and added it. I haven't uploaded it yet as I am adding more stuff that will be tested together.

Garados007 commented 1 year ago

@t-b Ready for next round. You can also checkout https://docs-staging.byte-physics.de/igortest/report/

t-b commented 1 year ago

ParseFilePath(5, winPath, "/", 0, 0) is not supported in Igor 9

Got it. To clarify, it does not work on Windows.

Review:

Much nicer!

Garados007 commented 1 year ago

The sed logic for the generate_reports CI job looks a bit scary.

This might be but is required because Igor is not intended to run under Linux. It does only see fake Windows paths and do not understand the symbolic links and therefore is the output a bit different compared to running this under Windows. The sed statements translates the fake Windows paths back to their real paths so that ReportGenerator can find the files.

I have seen no reason to build-in such translation into IUTF since it's only a CI limitation which can easily be fixed with sed magic.

After all. Suggestions are fixed and ready for review.

t-b commented 1 year ago

Very nice!! 💯