-
Notifications
You must be signed in to change notification settings - Fork 3
feat(ci): add coverage workflow #326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sdk-bindings
Are you sure you want to change the base?
Conversation
| with: | ||
| size: 256G | ||
|
|
||
| - name: Get the IOTA testnet binary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use the start_local_network action? If we don't need the gas station we could just add a flag to the inputs to control that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because coverage needs a self hosted runner and the action leaves a lot of stuff behind that would be annoying for future runs, the local network script takes care of reverting everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change the action then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk, not sure it's worth it. I could make an attempt at the action using the localnet script but I'm not sure we gain anything from it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nice to not have two places to change if we need to fix something
| echo "Scanning for corrupted .profraw files. This might take a while." | ||
| find target/llvm-cov-target -name '*.profraw' | while read file; do | ||
| if ! "$LLVM_PROFDATA" show "$file" > /dev/null 2>&1; then | ||
| echo "$file: corruped -> removing" | ||
| rm "$file" | ||
| fi | ||
| done | ||
|
|
||
| echo "Trying to merge .profraw files." | ||
| find target/llvm-cov-target -name '*.profraw' -print0 | xargs -0 $LLVM_PROFDATA merge \ | ||
| --failure-mode=warn \ | ||
| --sparse \ | ||
| --output target/nextest.profdata | ||
|
|
||
| if [ -s "target/nextest.profdata" ]; then | ||
| echo "Successfully collected nextest coverage data." | ||
| else | ||
| echo "🚨 Error 🚨: Collecting nexttest coverage failed." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably won't need that for the sdk tbh, at least I would try a few times without this. The problem in the monorepo was that running the code coverage on top of the tests often overpowered the runner's memory, and it was non-deterministic, so we needed this to make the coverage generation stable. In Sui they opted for just folding (in the sense of giving up and letting it fail). In our case I think the problem was more severe (less powerful hardware, or maybe also due to the transition from Stardust).
The SDK is much much smaller. I think creating coverage should almost never fail. We can even try to get the branch coverage analysis, not just only the line coverage working here. But that was too much for our runners against the monorepo 😿
Closes #288