-
Notifications
You must be signed in to change notification settings - Fork 12
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 some more subnet contract tests #227
Add some more subnet contract tests #227
Conversation
…hat only token owner can transfer
Codecov Report
@@ Coverage Diff @@
## master #227 +/- ##
===========================================
+ Coverage 0 93.47% +93.47%
===========================================
Files 0 6 +6
Lines 0 337 +337
===========================================
+ Hits 0 315 +315
- Misses 0 22 +22
... and 5 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it 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.
Before I go through all of the details in subnet_test.ts, could you separate out those new assertions into separate tests, so each one is a separate Clarinet.test
? This makes quickly reviewing the results of the test much simpler.
core-contracts/README.md
Outdated
@@ -0,0 +1,21 @@ | |||
# Subnets Core Contracts | |||
|
|||
Essential Clarity contracts for using Stacks Subnets |
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 like the idea of putting a README in here. Let's expand it a bit.
Essential Clarity contracts for using Stacks Subnets | |
This directory contains the contracts published to the Stacks L1 to implement a subnet. | |
* _subnet.clar_: interface between the subnet and the L1 | |
* _multi-miner.clar_: implements a multi-miner for the subnet | |
* _helper/*_: used for testing |
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 wasn't sure whether to do this or not because it takes a decent amount of setup to get to the point where I can attempt these transactions, but yeah I can make this change
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.
Yeah, I think it's worth it, since these tests don't take long to run.
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 was the code duplication that I wanted to avoid. Anyway, I did split most of my new tests out into separate tests. Also fixed all the remaining deno lint
warnings in the subnet_test.ts file
Co-authored-by: Brice Dobry <brice@obycode.com>
1664a5a
to
409b9be
Compare
Go ahead and remove the draft status on this when you're ready for a review. Is it ready now? |
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.
This is great. Just one minor comment.
Description
Added some failing tests for the
subnet.clar
contract. Some examples include: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