-
Notifications
You must be signed in to change notification settings - Fork 249
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
feat: adding llvm source based coverage #1236
Conversation
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
@frol this is ready for review :) |
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.
@jrmncos Could you, please, also provide a link for a CI test run on your fork? It would be great to see that it fully works before we merge it into the master (I'd like to avoid us debugging CI in production)
- name: Install Homebrew | ||
run: | | ||
/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" | ||
- name: Install LLVM | ||
run: brew install llvm |
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 will take too much time on every CI run. Can we use Linux as a host? It will be much faster to install it there.
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.
in this repo: https://github.com/near/near-sdk-rs/actions/runs/10295407665/job/28495033969?pr=1236
in my fork: https://github.com/jrmncos/near-sdk-rs/actions/runs/10295407683/job/28495034208?pr=1
it's takes ~8 minutes approximately. Let's try with Linux to reduce 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.
hey @frol I tried with ubuntu, and the longer step, that is 'Generate code coverage' it takes 7m:48seg https://github.com/near/near-sdk-rs/actions/runs/10360531522/job/28679132518
and with mac os 5m 36s
https://github.com/near/near-sdk-rs/actions/runs/10360643219/job/28679480453?pr=1236\
Even with the installation of Brew the total time of macos is still less. Also, the longer step in the GitHub actions pipelines it's Test core and it takes > 8m (this is the total time of coverage)
I tried using - uses: Swatinem/rust-cache@v1
as cache but has not improved the total time
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.
<10 minutes for code-coverage is not that bad. Let's see it in action!
@race-of-sloths invite
@jrmncos Thank you for your contribution! Your pull request is now a part of the Race of Sloths! Current status: executed
Your contribution is much appreciated with a final score of 2! Another weekly streak completed, well done @jrmncos! To keep your weekly streak and get another bonus make pull request next week! Looking forward to see you in race-of-sloths What is the Race of SlothsRace of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! Bot commands
|
🥁 Score it! @frol, please score the PR with |
#1174
Adding coverage to all directories declared in members excepting /examples
The changes were tested locally and the file generated by llvm-cov (llvm-cov-output.lcov) can be parsed as html and it's displaying the coverage correctly. Also the step Generate code coverage is running successfully.
As a final step to fully test this change is to setup the variable ${{ secrets.CODECOV_TOKEN }} in the repo settings. More info here: https://docs.codecov.com/docs/quick-start#getting-started If someone can set that variable it would be great.
I did it in my forked repo, and I received an email with something like
Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.
BTW, I have been dedicating most part of my time and effort to test the directory /examples with the tool wasmcov #1233 . I was able to generate the .profdata files for each directory, and maybe the next step it's to merge it and to integrate it with the changes of this PR. I'm working with @bbarwik was helping me to integrate the tool and I'm waiting for the next release that fix some bugs.
I would like to close this one and to continue working on #1233 with an accurate plan to ingrate /examples directory
@race-of-sloths include