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
Add property-based tests for signers-voting contract functions #4403
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #4403 +/- ##
==========================================
- Coverage 83.41% 83.40% -0.02%
==========================================
Files 448 448
Lines 323930 323930
==========================================
- Hits 270197 270158 -39
- Misses 53733 53772 +39 see 21 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
The tests for read-only functions look good.
Maybe variation over account is not necessary, as these are read-only functions that do not use tx-sender.
For a different PR, it might be interesting to check is-in-prepare-phase
with block-height in a contract call vs read-only call.
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 left a couple of suggestions.
Also, would you consider adding a formatter (such as prettier)?
Happy to help on that
contrib/core-contract-tests/tests/pox-4/signers-voting.prop.test.ts
Outdated
Show resolved
Hide resolved
contrib/core-contract-tests/tests/pox-4/signers-voting.prop.test.ts
Outdated
Show resolved
Hide resolved
Thanks for the review!
I always run |
Any formater is good as far as i'm concerned. But I was a bit surprised with this formatting pox_4_info.value.data[
"first-burnchain-block-height"]; |
me too, actually. It's left from when I was adding |
- reward-cycle-to-burn-height - burn-height-to-reward-cycle - is-in-prepare-phase This effort, initially part of #4286, is now in a separate commit and PR as it's autonomous with regards to PoX-4 property and fuzz testing. Additional property-based tests for the remaining functions of the signers-voting contract are warranted and will be addressed in separate commits/PRs.
03cfded
to
1b08cae
Compare
Addressed feedback, formatted using |
- Bump @stacks/transactions from ^6.9.0 to ^6.12.0 for `isClarityType`. - Use `assert` and `isClarityType` for stricter TypeScript type checks. - Replace `// @ts-ignore` with precise assertions, improving tests. Thanks to Hugo Caillard for his crucial help and guidance.
1b08cae
to
6e6090e
Compare
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.
LGTM
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.
Description
Add property-based tests for signers-voting contract functions
This effort, initially part of #4286, is now in a separate commit and PR as it's autonomous with regards to PoX-4 property and fuzz testing. Additional property-based tests for the remaining functions of the signers-voting contract are warranted and will be addressed in separate commits/PRs.
Applicable issues
Additional info (benefits, drawbacks, caveats)
Checklist
docs/rpc/openapi.yaml
andrpc-endpoints.md
for v2 endpoints,event-dispatcher.md
for new events)clarity-benchmarking
repobitcoin-tests.yml