protostar
protostar copied to clipboard
Support code coverage
https://github.com/starknet-edu/cairo-coverage
One of our interns at Nethermind @machard is interested in adding this to protostar. He's already got a minimal MVP by instrumenting function entry points with hints:
@kasperski95 mind if he takes it on?
@JorikSchellekens Yes, he can take this task
@machard Hey, can you tell us in a few points how do you plan to implement this feature? I'm asking because, we'd like to minimize merge conflicts.
cc @mkaput
@machard Not only to minimize merge conflicts, we would just like to be aligned with the ongoing work in test runner 😄 Its internals are not documented anywhere, and we have our own in-house intuition and plans for this.
For example, I'm currently working on an ability to configure test cases in Cairo code from a new kind of hook functions called setup cases (for test_my_test_name
the setup case would be called setup_my_test_name
). I guess you might be interested in adding an ability to configure coverage test-by-test in setup cases (doesn't have to happen, though! 🙂). My work includes a sizeable refactoring of test case runner internals, and it might be worth it to synchronize both pieces of work.
Hi,
So far it's a standalone PoC that you can find here https://github.com/machard/protostar-coverage-poc
- it parses the Cairo file to AST
- count some stuff in the AST
- instrument the AST and expose everything through
context
- write a new instrumented file
- there is a cheat code currently : the test is responsible to display the coverage result available in
context
I haven't looked into protostar codebase yet. But I was thinking:
- Protostar probably parse cairo file to AST ? In this case I was thinking to instrument on the fly here
- Protostar has access to
context
, so I was thinking to display coverage info at the same time protostar displays "Tests suite: x/x..."
I think the conflicting areas would be small. However, I do see potentially annoying stuff to reconcile lines numbers when an error happens in a test ⚠️ . Not sure exactly how to handle that yet.
However, there's also the possibility to continue to work on it as standalone tool for now.
It's still very much early stage. For example I haven't tried the instrumentation on various codebases and use cases. Also it currently just counts functions and statements executed.
So I guess what you can tell me:
- Do I keep it as a standalone thing for now, or do I do a PoC integration into protostar to check if my idea of integration is ok ?
- Should I focus for now on having the standalone PoC running on any kind of codebase/usecase ? in that case maybe you have a list of codebases to try it out ?
- Should I try to see if some other feature is doable ?
- Any other thing you want 😃
- Protostar probably parse cairo file to AST ? In this case I was thinking to instrument on the fly here
Yes, we do even modify the Cairo compiler with our own compilation passes. You should be easily able to inject your Instrument
visitor there.
https://github.com/software-mansion/protostar/blob/6048e1ba3553aa264f9a8c9e9b923a4651484ab0/protostar/utils/compiler/pass_managers.py
- Protostar has access to
context
, so I was thinking to display coverage info at the same time protostar displays "Tests suite: x/x..."
We have an object called ExecutionState
and test-specific subclass called TestExecutionState
. Your context can easily be put there as a field and then used from cheatcodes and thus -- available to instrumentation hints.
I do a very similar thing in the yet in progress max_examples
cheatcode. You can peek the WIP commit here: https://github.com/software-mansion/protostar/commit/5510f43c33b28fc8a700b4d531795ac992aff58f
However, I do see potentially annoying stuff to reconcile lines numbers when an error happens in a test ⚠️ . Not sure exactly how to handle that yet.
Just do not manipulate the location
field in all AST structures, and use the location of CodeElementFunction
as the location of your injected hints. That's what cairo-lang
does itself, for example in the if a and b
syntax 😉
I think the conflicting areas would be small.
With your approach, it looks like so! FYI The results of coverage can be put in the ExecutionResourcesSummary
class, instead of printing these to stdout.
In addition, you might research hooking into our subclass of Cairo VM which we use in tests: CheatableVirtualMachine
. At this point, you have access to any Cairo instruction that is executed in the program. I don't know how much about the program structure you can reconstruct though in this scenario, because you are now effectively observing CASM (Cairo Assembly).
Nonetheless, I wonder how much of this is already done by Cairo tracer. @MaksymilianDemitraszek is currently very slowly working on a Cairo profiler which uses the mentioned utility. He wrote down his observations here: https://github.com/software-mansion/protostar/discussions/542. I would like to hear from Maks his opinion about this.
In general, if Maks doesn't see anything against, you have a green light from me to work on this directly in Protostar. Let's hear the voices from the rest of the team, though. We need to think more how to organize the work.
@machard Thanks for taking interest in contribution to our project! I am more than excited for this feature. ❤️
As @mkaput said I am currently working on the cairo profiler. I believe using hints for your problem might be, great and very creative, but not the best approach - in terms of maintainability and simplicity.
What I would suggest is using cairo trace and debug info which would provide you the mapping from assembly to instructions/functions lines and let you deduce covered code.
Files which may be interesing to you:
- https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/lang/tracer/tracer_data.py
- https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/lang/tracer/profiler.py
I would be more than happy to assist or help you with any matter regarding this feature or cairo trace/cairo vm.
If you need any help feel free to write to me on Starknet discord.
for posterity, as i'll focus on learning cairo, i'm leaving that here : https://github.com/software-mansion/protostar/compare/master...machard:coverage-poc?expand=1 in short, with the current codebase, the printed lines seems to contains correct locations for test files and files imported directly from the test files. However lines referring to files deployed with deploy_contract will be referring to the "autogen" code, which is not what we want.
We will investigate integrating https://github.com/starknet-edu/cairo-coverage into Protostar