Skip to content

Macro code coverage tool#15738

Merged
straight-shoota merged 14 commits intocrystal-lang:masterfrom
Blacksmoke16:macro-code-coverage
Jun 26, 2025
Merged

Macro code coverage tool#15738
straight-shoota merged 14 commits intocrystal-lang:masterfrom
Blacksmoke16:macro-code-coverage

Conversation

@Blacksmoke16
Copy link
Member

@Blacksmoke16 Blacksmoke16 commented May 2, 2025

Intro

This PR represents an initial implementation of a new compiler tool to generate a https://docs.codecov.com/docs/codecov-custom-coverage-format coverage report for macro code. Functionally, the tool feels very robust, with a ton of spec coverage. I tested it out on the various macro code within Athena and I'm at the point now where I'm satisfied with how it's working to the point where I feel comfortable opening this PR.

I've created the PR as a draft as it depends on some other PRs, and because it intentionally includes debug code commented out. The intent/goal around this PR is to mainly show off the implementation thus far, and to solicit feedback either in the form of other changes that would make this PR's implementation simpler, or general high level tweaks.

Please go try it out on your own project(s) to see if you can find any problems/issues.

Implementation Overview

At a high level the current implementation adds a new crystal tool that when executed enables a new collect_covered_macro_nodes property on the program. When enabled, the macro interpreter will collect macro AST nodes via the #collect_covered_node method. Because a new macro interpreter is used for each expansion, the collected nodes are "flushed" to another array pending processing via the tool after each macro is expanded.

The tool itself iterates over each "group" of collected nodes from each expansion, skipping any that do not relate to user code. It then iterates over reach "group", chunking them based on the line number. The chunking ensures that each node can increment that line's count by 1 or 0, not how many nodes are on that single line.

From here, we then perform some heuristics to determine if a given line was a hit, partial hit, or a miss. Partial hits are kept track by caching the specific conditionals and if it was a hit/miss on a line so we can know how many paths of the total possibilities we hit on that single line.

Macro raise is handled by raising a new SkipMacroCodeCoverageException that extends SkipMacroException. Rescuing this exception allows the tool to run on the code that was collected up to before the exception was raised, while not resulting in an error/running all the other code after it. The exception's message/trace is printed to STDOUT STDERR while the report is written to STDERR STDOUT. This is esp useful for testing error flows of custom macro logic as it allows the code to assert the proper error was raised, while still allowing to save the coverage report. I.e. the report JSON/exception outputs are not co-mingled. The main benefit of this is preventing the need to run these kind of tests twice, once for the coverage report and once for the actual test assertions.

Depends on #15709

Resolves #14880

Open Issues

Will keep a running list of additional things that need handled:

  • CI failing on Crystal 1.0
  • Non-user raised exceptions being swallowed

Surface non-user raised exceptions (type errors, etc)
Workaround Crystal 1.0 compiler bug
Cleanup code for review
@Blacksmoke16 Blacksmoke16 marked this pull request as ready for review May 31, 2025 14:09
@Blacksmoke16
Copy link
Member Author

Pushed up another commit to remove the debug code and such. Haven't heard any preliminary feedback yet, so going to mark this as ready for review!

Copy link
Member

@RX14 RX14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do a detailed line-by-line review, but the specs look good and the approach looks well done to me.

Copy link
Collaborator

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! ❤️

As stated in a comment below, I wonder if the feature could be pluggable, with most of the implementation living in the tool. For example through a simple Proc.

The only impact to the compiler would be a single property for the callback, and of course calling it when defined in the macro interpreter, then everything else would live in the macro code coverage tool.

@Blacksmoke16
Copy link
Member Author

Pushed up a couple more commits after testing the integration into the spec component would be like. Found/fixed one bug, and realized it made more send errors to STDERR and the report to STDOUT to allow it to handle non-macro related exceptions.

Running all DI component specs on locally built compiler without this PR/integration code was ~43s, with it, it's ~50s. So not too far off.

Working branch: athena-framework/athena@master...macro-code-coverage

@straight-shoota straight-shoota added this to the 1.17.0 milestone Jun 23, 2025
Update code and add more specs
@straight-shoota straight-shoota merged commit 34875ff into crystal-lang:master Jun 26, 2025
36 of 38 checks passed
@Blacksmoke16 Blacksmoke16 deleted the macro-code-coverage branch June 26, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Macro Code Coverage

4 participants