Skip to content

Conversation

@Johnnycus
Copy link
Contributor

No description provided.

@Johnnycus Johnnycus linked an issue Nov 20, 2025 that may be closed by this pull request
Copy link
Contributor

@billythedummy billythedummy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i missed describing this in the issue but actly i'd wanted to refactor the tests of all the individual instructions to have a single universal test fn that takes few args. set_admin_test is a good example where it takes the bare min of the instruction, accounts and expected error. Some instructions (or instruction chain in rebalance's case) may need more args - for eg test fn for swap instructions should probably take a Quote as an arg that it can check against.

The reason for doing this is just bec i thought i'd landed on a nice clean understandable pattern there, but lmk if it isnt the case in your opinion or you have suggestions for improvement

We can do this in separate PRs, maybe 1 PR for each instruction

@billythedummy
Copy link
Contributor

Some instructions (or instruction chain in rebalance's case) may need more args - for eg test fn for swap instructions should probably take a Quote as an arg that it can check against

you can probably express different set of checks to run on success vs on error using a Result type lul like Result<Quote, ProgramError>

@Johnnycus
Copy link
Contributor Author

i missed describing this in the issue but actly i'd wanted to refactor the tests of all the individual instructions to have a single universal test fn that takes few args. set_admin_test is a good example where it takes the bare min of the instruction, accounts and expected error. Some instructions (or instruction chain in rebalance's case) may need more args - for eg test fn for swap instructions should probably take a Quote as an arg that it can check against.

The reason for doing this is just bec i thought i'd landed on a nice clean understandable pattern there, but lmk if it isnt the case in your opinion or you have suggestions for improvement

We can do this in separate PRs, maybe 1 PR for each instruction

i like that pattern, think we can stick with it. let's make a PR per group of ix tests like: admin, disable_pool, liquidity, protocol_fee, rebalance, swap, sync_sol_value?

@Johnnycus Johnnycus merged commit 501f68e into master Nov 21, 2025
11 checks passed
@Johnnycus Johnnycus deleted the 124-cleanup-strategy-centric-test-rewrite branch November 21, 2025 09:03
billythedummy pushed a commit that referenced this pull request Dec 5, 2025
* cleanup proptests with *_strat fns

* add common SetSvcBaseInputs struct in setsvc test
billythedummy added a commit that referenced this pull request Dec 5, 2025
* cleanup proptests with *_strat fns

* add common SetSvcBaseInputs struct in setsvc test

Co-authored-by: Albert Itayev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cleanup: Strategy-centric test rewrite

3 participants