-
Notifications
You must be signed in to change notification settings - Fork 18
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(code): Add Makefile to fix lints and test in local #751
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #751 +/- ##
==========================================
- Coverage 81.69% 81.67% -0.02%
==========================================
Files 182 182
Lines 16266 16266
==========================================
- Hits 13287 13284 -3
- Misses 2979 2982 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Makefile
Outdated
cd code && cargo clippy --fix --allow-dirty --workspace -- -D warnings | ||
|
||
test: | ||
cd code && cargo test |
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.
For running unit tests, I would advise using https://nexte.st as it's a much more capable test runner than the builtin test command.
For integration tests, we use cargo-maelstrom
to isolate each test and run them in parallel.
So let's perhaps add two targets: integration-tests
and unit-tests
and combine them under test
.
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.
See the corresponding GitHub Action workflows for more details:
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 will update the CONTRIBUTING.md file accordingly to better surface this information.
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 updated, but will add integration-tests, currently cargo-maelstrom
don't work well on MacOs
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.
Ah right, then we can instead use cargo nextest -p informalsystems-malachitebft-starknet-test --no-capture
which works on all platforms.
Closes: #XXX
PR author checklist
For all contributors
For external contributors