-
Notifications
You must be signed in to change notification settings - Fork 7
Add stx-defi contract and tests to examples
#170
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
Conversation
This allows running unit tests using `clarinet-sdk` for demonstration reasons.
cfeda32 to
c47b79b
Compare
stx-defi contract and tests to examples
|
As this PR is already fairly-sized, a follow-up PR is needed. It should contain:
|
|
@moodmosaic Interesting finding implementing this PR: after the This means a user working with a basic Clarinet project and using Node 18 will not be able to easily use Rendezvous along with a js dialers file in the current setup. |
moodmosaic
left a 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.
The stx-defi contract addition is a solid move — it nicely surfaces how a PoX-2-style bug can be caught with rendezvous mechanics. That’s exactly the kind of thing these examples should be doing.
But: the TypeScript unit tests in example/ feel out of place. They're not in the same testing idiom we use for stx-defi (no fast-check, lots of boilerplate), and they pull in a bunch of TS scaffolding that doesn’t align with the purpose of this folder(?)
Two options:
- Drop the TS tests, and just document the behavior they demonstrate alongside the Clarity code.
- Or, move them into a dedicated folder (name of that would be?) and rewrite to match the Clarity-from-TypeScript testing pattern (leaner, property-based, more focused).
As-is, they dilute more than they add.
|
It's better if we add the |
@moodmosaic This could end up hiding configuration issues. Since Rendezvous is designed to work with a Clarinet project, I can’t imagine a real use case where someone would use it outside of one. That’s what happened in #174 as well. Our example folder should match a real Clarinet project as closely as possible. Otherwise, users learning Rendezvous might get confused by an unusual folder structure. |
|
Yes, indeed. Perhaps all we need is a good |
moodmosaic
left a 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.
LGTM 💯
This PR adds the stx-defi contract to the examples, along with corresponding unit tests and newly added Rendezvous tests that can find a bug inside the contract.
Updates:
examplefolderpackage.json,tsconfig.json,vitest.config.jsturning the folder into a Clarinet project; this allows runningsimnetunit testsexamplefolder exclusion from Rendezvoustsconfig.json.cjsfor compatibility with the current format of the example and the supported Node versions and all the references to this fileResolves #171 as a side effect.