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
Test coverage #4596
Test coverage #4596
Conversation
export LEMMY_CONFIG_LOCATION=../../config/config.hjson | ||
export RUST_BACKTRACE=1 | ||
|
||
cargo install cargo-llvm-cov |
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.
Not sure if this cargo install
should be here
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.
Seems to be okay, its not doing a reinstall.
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.
If this ever gets run from CI, I'd recommend cargo binstall
like we have with some of the others.
scripts/test-with-coverage.sh
Outdated
export RUST_BACKTRACE=1 | ||
|
||
cargo install cargo-llvm-cov | ||
cargo llvm-cov --workspace --all-features --no-fail-fast --lcov --output-path lcov.info |
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.
Im not sure how to open the lcov file. Why not use --html --open
?
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.
Lcov file can be used to show coverage in the code editor (e.g. with coverage gutters extension for vscode)
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.
Would be good if you explain that in a comment. I tried the gutters extension but it fails to find the coverage file, even after I manually configure it to use lcov.info. Can you share your plugin config?
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.
I'm using the default config.
Did you click "Watch" at the bottom and open a Rust file? If it says "No Coverage" then that's just referring to the current file.
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.
Added comment
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.
After clicking "watch" there is an error message "Could not find a Coverage file! Searched for lcov.info, cov.xml, coverage.xml, jacoco.xml, coverage.cobertura.xml". But lcov.info definitely exists and the open file has various tests, so no idea what the problem is. Im using VSCodium in case that makes a difference.
It might also be interesting to consider api tests for coverage. Should be possible with |
.gitignore
Outdated
@@ -34,3 +34,6 @@ dev_pgdata/ | |||
|
|||
# database dumps | |||
*.sqldump | |||
|
|||
# test coverage | |||
lcov.info |
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.
I would put this in the target folder.
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.
Done
export LEMMY_CONFIG_LOCATION=../../config/config.hjson | ||
export RUST_BACKTRACE=1 | ||
|
||
cargo install cargo-llvm-cov |
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.
If this ever gets run from CI, I'd recommend cargo binstall
like we have with some of the others.
Ability to notice untested code can increase the chance of fixing mistakes like the one noticed in #4560