-
Notifications
You must be signed in to change notification settings - Fork 214
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
chore(acceptance): add tests for PSM to z:acceptance
#10273
Conversation
b167d7d
to
0c1ae50
Compare
Deploying agoric-sdk with Cloudflare Pages
|
This comment was marked as resolved.
This comment was marked as resolved.
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 did a code quality pass. I don't have PSM swapped in enough rn to evaluate whether this fully closes the issue.
324d3ee
to
712f970
Compare
Addressed all change requests outside of #10273 (comment). Should I ask for a re-review once the last one is done as well? @turadg |
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.
Code LGMT. I haven't reviewed for whether this covers all the PSM testing requirements.
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 meant to separate the critical comments from the suggestions, but it's taking too long.
Consider these all suggestions. I'll do another pass to look for anything critical.
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 cover the 6 requested items (New user can complete a swap, ...).
This approval presumes you'll address any lint failures, as usual. No need to re-request review once you've done that.
I found it a little challenging to navigate the split between psm.test.js
and psm-lib.js
in some places. I made some suggestions about factoring. Those are suggestions; feel free to act on them, respond to them, or ignore them, at your discretion.
d50ed58
to
d48faaf
Compare
Refs: Agoric/BytePitchPartnerEng#23 fix(acceptance-psm): address change requests
d48faaf
to
11b336b
Compare
closes: https://github.com/Agoric/BytePitchPartnerEng/issues/23
Description
Current version of
z:acceptance
tests do not cover PSM at all. The goal of this PR is to make sure we cover test cases in https://github.com/Agoric/BytePitchPartnerEng/issues/23 prepared by @otoole-brendanSecurity Considerations
None.
Scaling Considerations
None.
Documentation Considerations
None.
Testing Considerations
The tests assume that
anchorPerMinted
ratio is 1. I think this is a safe assumption because;boot
tests for the PSM has the same assumptionUSDC_axl
instance which is something we shouldn't worry about anytime soon.Upgrade Considerations
None.