CI: Get coverage of filecheck tests

[webmiche]

This PR leverages the Coverage package to generate the code coverage of our file checks test suite. Currently, there are a couple of choices that are controversial, which I would like to hear your perspectives. I will add comments to the corresponding locations in the file.

[webmiche]

Notice that this currently does not include the coverage of tests that depend on the MLIR Pyhton bindings as the coverage is uploaded for the Python-based testing only. We might want to consider automatically updating the coverage once the MLIR-based testing has finished.

[codecov]

Codecov Report

Base: 81.32% // Head: 77.80% // Decreases project coverage by -3.52% ⚠️

Coverage data is based on head (ce5ea46) compared to base (4d93517).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #256      +/-   ##
==========================================
- Coverage   81.32%   77.80%   -3.53%     
==========================================
  Files          31       38       +7     
  Lines        5510     6023     +513     
  Branches      938     1045     +107     
==========================================
+ Hits         4481     4686     +205     
- Misses        786     1082     +296     
- Partials      243      255      +12     

Impacted Files	Coverage Δ
xdsl/xdsl_opt_main.py	0.00% <0.00%> (ø)
xdsl/irdl.py	83.67% <0.00%> (ø)
xdsl/dialects/cmath.py	0.00% <0.00%> (ø)
xdsl/mlir_converter.py	66.37% <0.00%> (ø)
xdsl/dialects/affine.py	62.79% <0.00%> (ø)
xdsl/dialects/llvm.py	75.00% <0.00%> (ø)
xdsl/irdl_mlir_printer.py	0.00% <0.00%> (ø)
xdsl/dialects/cf.py	0.00% <0.00%> (ø)
xdsl/dialects/builtin.py	80.44% <0.00%> (+0.24%)	⬆️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[math-fehr]

Yes, I like the direction this is getting!

[webmiche]

Okay, I think I have a design that somehwat makes sense now. Considering that lit seems to be weird around the exec_root thing, I decided to dodge it. I have instead defined two user flags for lit:

• "-DCOVERAGE" makes filecheck tests generate coverage data
• "-DCOVERAGE_CONFIG" can be passed to specify the coverage config (in our case xdsl/.coveragerc). I then set this using the absolut path from within the CI, therefore sidestepping the execution root issues.

This also means that the coverage files get created next to the filecheck files. Therefore, the coverage combine command got a bit more involved, specifying all subfolders. If anyone sees a nicer way of doing this, tell me pls :)
I have look into the using **, but wasn't able to get something to work that takes arbitrarily deeply nested subdirectories into account.

The aforementioned flags are passed to xdsl-opt in two steps:

• firstly, xdsl-opt now has two arguments --generate-coverage and --coverage-config[=PATH], which correspond basically one-to-one to the aforementioned two flags.
• Secondly, I use custom substitutions in lit in order to substitute %cov to either "" (no coverage flag), --generate-coverage (only DCOVERAGE) or --generate-coverage --coverage-config=[PATH] (both flags).

[math-fehr]

Looks really nice now!
I added some comments, to see if we can make this a bit less error-prone for when we will update it, but that's really cool!

[math-fehr]

Is this here where you can't use ** ?
If this is the case, can we write a small script that automates this instead?

[webmiche]

Changed to coverage combine --append $(find tests/filecheck -type d).

[math-fehr]

Looks good to me!
Should we merge this now, or do you still want to add something else to it?

[webmiche]

I want to make sure that the code coverage is properly uploaded since I'm not 100% sure how this exactly works.

[webmiche]

Yes, as somewhat expected/feared the .xml file was not found to upload. I will look into this on my way home, hoping I can fix it in time. O/w, we might have to wait until monday to merge this.

If I succeed, I will just merge :)

[webmiche]

This is extremely weird. If I hard code the path to the file, it works. However, if I try to use the environment variable (so how a sane person would do it), the file is not found. Currently, I am trying to not pass a path and hope it works magically.

If not, I will probably look into this again on Monday and merge it then...

[webmiche]

Well, it found everything, only the one it should have is missing...

[math-fehr]

Thanks a lot for all this work. Personally, I don't really know how these things work, but I can take a look if you want!

[webmiche]

I think I am getting closer. Now, the file is discovered, but it seems that some of the tests are not taken into account. That's why the coverage drops by so much...

[webmiche]

Noting for my future self: There are two issues atm: Both lit and pytest act like they didn't have the Python Bindings installed. Additionally, the ${GITHUB_WORKSPACE} env variable seems weird, and pytest hsa the wrong root directory.

[webmiche]

Okay, I think this is about to get finished. I had one run earlier that behaved the way I wanted it too. Notice that we now lose some coverage as I have specified the source directories, so non-executed files are counted as 0% coverage. Furthermore, coverage is now generated on the MLIR CI.

Side thought: We might want to think about what to do with all the __init__ files, as they count towards coverage now, but actually aren't possible to cover 100% within one CI run.