Implement tests according to missing test list #58 #61

Nerrolol · 2024-03-20T19:50:06Z

Implementing a first batch of airdrop tests

i'm not sure about the hash to use for the root or the proofs (shall I use the same as those in the defi repo ?)
for the test_update_root_and_claim_attempts function i still have to check the fail, not sure of the behaviour of the should_panic function
finally there is still the check claimed event test to be implemented which I haven't digged yet, maybe there is a further environment to implement to do this one ?

also the valid claim attempt is tested on the test_update_root_and_claim_attempts function

tensojka

We want to be testing the Governance contract itself, not the Carmine deployment of it.

The setup.cairo should Deploy the contract, distribute tokens to a few addresses, and then have these addresses vote.

The reason is that the tests must be testing the current version in master, in some branch, not the currently deployed version – tests need to catch issues before we deploy with those issues

Also, please run scarb fmt

Lacks setting of custom proposal configuration during deployment

…pgrade root and passes it

…te to sample addresses and then vote on update root proposal

* add: basic ui displaying proposals * feat: voting invocations * feat: ✨ new proposal form * feat: ✨ use async for proposal creation * fix: 🐛 bad voting code * chore: 🗑️ cleanup * feat: ✨ add logo * fix: 🐛 wrong site title --------- Co-authored-by: Tomáš Hobza <[email protected]>

Co-authored-by: Tomáš Hobza <[email protected]>

* initial commit * change to testnet * Add basic working event printing * notification telegram bot working * add gitignore * Remove node_modules folder and update .gitignore * update config.toml * removing unused address * Delete accidentally committed tests/proposal.cairo * code review changes * rest of the code-review changes * Update packages according to code review feedback * Add error print when unable to send messages to Telegram * Add automatic retrieval of chain id * Polish formatting * Refactor config file reading * code-review changes in index.ts, package-lock * Add CI runner * Trigger CI * Fix CI * Fix CI * Fix CI * Fix CI --------- Co-authored-by: Ondřej Sojka <[email protected]> Co-authored-by: Ondřej Sojka <[email protected]>

tensojka

Can you rebase on top of master? I see changes such as frontend and notification-bot in the Changes tab.

I don't think the deploy_and_distribute_gov_tokens function is okay since both test_proposal_expiry and test_vote_on_expired fail with 'Failed to deserialize param'

tensojka · 2024-05-20T15:09:44Z

src/testing/setup.cairo

+fn deploy_and_distribute_gov_tokens(recipient: ContractAddress) -> IERC20Dispatcher {
+    let mut calldata = ArrayTrait::new();
+    calldata.append(GOV_TOKEN_INITIAL_SUPPLY);
+    calldata.append(recipient.into());
+
+    let gov_token_contract = declare("FloatingToken");
+    let token_addr = gov_token_contract.deploy(@calldata).expect('unable to deploy FloatingToken');
+    let token: IERC20Dispatcher = IERC20Dispatcher { contract_address: token_addr };
+
+    start_prank(CheatTarget::One(token_addr), admin_addr.try_into().unwrap());
+
+    token.transfer(first_address.try_into().unwrap(), 100000);
+    token.transfer(second_address.try_into().unwrap(), 100000);
+    token


What are you trying to do here? You can't just transfer the tokens from the token address as on deployment, there aren't any. Maybe you wanted to use mint?

tensojka · 2024-05-20T15:10:31Z

tests/proposals_tests.cairo

+    assert!(dispatcher.get_proposal_status(prop_id) == 1, "proposal not passed!");
+}
+
+#[should_panic]


Instead of a generic 'should panic', you should also put the expected error message here.

https://foundry-rs.github.io/starknet-foundry/testing/testing.html?highlight=should_#expected-failures

tensojka · 2024-05-20T15:22:27Z

tests/proposals_tests.cairo

+    let end_timestamp = current_timestamp + constants::PROPOSAL_VOTING_SECONDS;
+    start_warp(CheatTarget::One(gov_contract_addr), end_timestamp + 1);
+
+    let status = dispatcher.get_proposal_status(prop_id);


Need to add check that status is -1 (or 2? not sure) meaning it failed.

* add: basic ui displaying proposals * feat: voting invocations * feat: ✨ new proposal form * feat: ✨ use async for proposal creation * fix: 🐛 bad voting code * chore: 🗑️ cleanup * feat: ✨ add logo * fix: 🐛 wrong site title --------- Co-authored-by: Tomáš Hobza <[email protected]>

Co-authored-by: Tomáš Hobza <[email protected]>

* Add mdbook stub * Update license to Apache 2.0 * Add some docs

* Add custom proposal implementation Lacks setting of custom proposal configuration during deployment * Add arbitrary proposal * Polish apply_passed_proposal * Polish scarb fmt * Cherry pick setup.cairo from 1fab54f * Move test setup to src/ * Remove unrelated functions from setup.cairo * Fix setup * Polish tests * Fix import naming * Add addition of custom proposal

* Update Scarb.toml, add [lib] * Add custom proposal implementation Lacks setting of custom proposal configuration during deployment * Add arbitrary proposal * Polish apply_passed_proposal * Polish scarb fmt * Cherry pick setup.cairo from 1fab54f * Move test setup to src/ * Remove unrelated functions from setup.cairo * Fix setup * Polish tests * Fix import naming * Add addition of custom proposal * Remove part of Carmine code * Remove rest of Carmine stuff * Polish with scarb fmt * Bump version to 0.4.0 * Fix imports in tests * Remove unused test from lib.cairo * Remove cubit as dependency * Update package description * Update error message to not mention CARM * Move snforge_std to dev-dependency * Update snforge_std to v0.23.0 * Update snforge in CI * Trigger CI * Format

… them in deploy token function

* Add custom proposal implementation Lacks setting of custom proposal configuration during deployment * Add arbitrary proposal * Polish apply_passed_proposal * Polish scarb fmt * Cherry pick setup.cairo from 1fab54f * Move test setup to src/ * Remove unrelated functions from setup.cairo * Fix setup * Polish tests * Add airdrop tests * Adding setup file with generic function which applies a proposal to upgrade root and passes it * Setup environment to deploy governance, deploy gov token and distribute to sample addresses and then vote on update root proposal * Add tests on proposals * fix some issues on proposal tests * Proposals Frontend (#66) * add: basic ui displaying proposals * feat: voting invocations * feat: ✨ new proposal form * feat: ✨ use async for proposal creation * fix: 🐛 bad voting code * chore: 🗑️ cleanup * feat: ✨ add logo * fix: 🐛 wrong site title --------- Co-authored-by: Tomáš Hobza <[email protected]> * Update README.md * Update README.md * feat: default to_upgrade value (#69) Co-authored-by: Tomáš Hobza <[email protected]> * Telegram Notification bot for proposal (#71) * initial commit * change to testnet * Add basic working event printing * notification telegram bot working * add gitignore * Remove node_modules folder and update .gitignore * update config.toml * removing unused address * Delete accidentally committed tests/proposal.cairo * code review changes * rest of the code-review changes * Update packages according to code review feedback * Add error print when unable to send messages to Telegram * Add automatic retrieval of chain id * Polish formatting * Refactor config file reading * code-review changes in index.ts, package-lock * Add CI runner * Trigger CI * Fix CI * Fix CI * Fix CI * Fix CI --------- Co-authored-by: Ondřej Sojka <[email protected]> Co-authored-by: Ondřej Sojka <[email protected]> * Adding proposals tests * Comment code for airdrop_tests and remove old setup file * Add minor fixes towards compilability * Fix all proposals tests * Fix formatting * Proposals Frontend (#66) * add: basic ui displaying proposals * feat: voting invocations * feat: ✨ new proposal form * feat: ✨ use async for proposal creation * fix: 🐛 bad voting code * chore: 🗑️ cleanup * feat: ✨ add logo * fix: 🐛 wrong site title --------- Co-authored-by: Tomáš Hobza <[email protected]> * feat: default to_upgrade value (#69) Co-authored-by: Tomáš Hobza <[email protected]> * Update frontend domain to konoha.vote * Rename package from 'governance' to 'konoha' * Add contributor guidelines * Update README.md with contributing info * Update CONTRIBUTING.md * Add mdbook scaffolding (#75) * Add mdbook stub * Update license to Apache 2.0 * Add some docs * Add custom proposal implementation (#64) * Add custom proposal implementation Lacks setting of custom proposal configuration during deployment * Add arbitrary proposal * Polish apply_passed_proposal * Polish scarb fmt * Cherry pick setup.cairo from 1fab54f * Move test setup to src/ * Remove unrelated functions from setup.cairo * Fix setup * Polish tests * Fix import naming * Add addition of custom proposal * Remove mentions of Carmine in this repository (#73) * Update Scarb.toml, add [lib] * Add custom proposal implementation Lacks setting of custom proposal configuration during deployment * Add arbitrary proposal * Polish apply_passed_proposal * Polish scarb fmt * Cherry pick setup.cairo from 1fab54f * Move test setup to src/ * Remove unrelated functions from setup.cairo * Fix setup * Polish tests * Fix import naming * Add addition of custom proposal * Remove part of Carmine code * Remove rest of Carmine stuff * Polish with scarb fmt * Bump version to 0.4.0 * Fix imports in tests * Remove unused test from lib.cairo * Remove cubit as dependency * Update package description * Update error message to not mention CARM * Move snforge_std to dev-dependency * Update snforge_std to v0.23.0 * Update snforge in CI * Trigger CI * Format * Rename governance:: to konoha:: * Fix proposals tests and mint tokens to admin addr before distributing them in deploy token function * Add quorum tests * Move setup to testing, fix tests so they compile * Merge branch master --------- Co-authored-by: Ondřej Sojka <[email protected]> Co-authored-by: Tomáš Hobza <[email protected]> Co-authored-by: Tomáš Hobza <[email protected]> Co-authored-by: Ondřej Sojka <[email protected]> Co-authored-by: xkrivan5 <[email protected]>

tensojka self-requested a review March 27, 2024 13:12

tensojka requested changes Mar 27, 2024

View reviewed changes

tensojka and others added 20 commits April 26, 2024 14:24

Add custom proposal implementation

d4071f4

Lacks setting of custom proposal configuration during deployment

Add arbitrary proposal

25ec5d5

Polish apply_passed_proposal

676a2e2

Polish scarb fmt

d178036

Cherry pick setup.cairo from 1fab54f

1850814

Move test setup to src/

4a308b9

Remove unrelated functions from setup.cairo

03d1ba5

Fix setup

82d7947

Polish tests

6ed9c58

Add airdrop tests

0ef0d64

Adding setup file with generic function which applies a proposal to u…

aa54ac2

…pgrade root and passes it

Setup environment to deploy governance, deploy gov token and distribu…

7f32f99

…te to sample addresses and then vote on update root proposal

Add tests on proposals

a2e7bfa

fix some issues on proposal tests

3f5f10d

Update README.md

0e0c0b7

Update README.md

c51f55a

feat: default to_upgrade value (#69)

04f6801

Co-authored-by: Tomáš Hobza <[email protected]>

Adding proposals tests

b144b85

Nerrolol force-pushed the missing-tests branch from 1fab54f to b144b85 Compare May 13, 2024 20:11

Nerrolol and others added 3 commits May 13, 2024 20:23

Comment code for airdrop_tests and remove old setup file

d4fdf9b

Add minor fixes towards compilability

2884f06

Fix all proposals tests

89f9028

tensojka requested changes May 20, 2024

View reviewed changes

tensojka and others added 3 commits May 20, 2024 15:25

Fix formatting

dc964b5

feat: default to_upgrade value (#69)

b7f718e

Co-authored-by: Tomáš Hobza <[email protected]>

tensojka and others added 14 commits May 23, 2024 11:44

Update frontend domain to konoha.vote

d622ff9

Rename package from 'governance' to 'konoha'

fc71439

Add contributor guidelines

4620dd0

Update README.md with contributing info

2a12029

Update CONTRIBUTING.md

32dbf37

Add mdbook scaffolding (#75)

7b4cb3c

* Add mdbook stub * Update license to Apache 2.0 * Add some docs

Rename governance:: to konoha::

eb1181d

Fix proposals tests and mint tokens to admin addr before distributing…

9f61980

… them in deploy token function

Add quorum tests

d837e28

Move setup to testing, fix tests so they compile

5135607

Merge branch master

9109ef3

Merge branch 'master' into missing-tests

37d590d

tensojka merged commit d8cc8e0 into master May 27, 2024
1 check failed

tensojka mentioned this pull request May 28, 2024

Finish Proposals tests #93

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement tests according to missing test list #58 #61

Implement tests according to missing test list #58 #61

Nerrolol commented Mar 20, 2024 •

edited

Loading

tensojka left a comment

tensojka left a comment

tensojka May 20, 2024

tensojka May 20, 2024

tensojka May 20, 2024

Implement tests according to missing test list #58 #61

Implement tests according to missing test list #58 #61

Conversation

Nerrolol commented Mar 20, 2024 • edited Loading

tensojka left a comment

Choose a reason for hiding this comment

tensojka left a comment

Choose a reason for hiding this comment

tensojka May 20, 2024

Choose a reason for hiding this comment

tensojka May 20, 2024

Choose a reason for hiding this comment

tensojka May 20, 2024

Choose a reason for hiding this comment

Nerrolol commented Mar 20, 2024 •

edited

Loading