-
Notifications
You must be signed in to change notification settings - Fork 355
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
Unit tests for cw3-fixed-multisig #95
Conversation
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.
Thank you for the start. Added some comments on how to simplify the test code, but also some points that need testing and likely don't work properly in the contract code, which I didn't see until reading these test cases. Thank you for them
Nice cleanup, and you convinved me about the 0 weight members. This should be documented in the README (TODO) |
0c40dbd
to
dd99863
Compare
bab274e
to
87a1fb2
Compare
87a1fb2
to
4d2c829
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.
Looking good, just those last TODOs and it is golden.
// non-Open proposals cannot be voted | ||
// Vote it again | ||
let env = mock_env(VOTER5, &[]); | ||
let res = handle(&mut deps, env, yes_vote); |
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.
After adding this test, I was left wondering that maybe it's a good idea to allow voting on Passed
proposals: https://github.com/CosmWasm/cosmwasm-plus/blob/f979892bb7d32339769ad83ccb289e92e5a2f300/contracts/cw3-fixed-multisig/src/contract.rs#L165
That way, we can get better statistics on the final result of a given proposal. That is, despite a proposal having passed, it can have a large number of No
/ Veto
votes (some of which could have been cast after it passed). Or, conversely, a proposal can have a larger adoption than the one reflected by the results; which currently "freeze" after it passes.
Thank you |
There are still some extra checks to do, but I think these now are acceptable unit tests.