This repository has been archived by the owner on Oct 6, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0xNeshi
force-pushed
the
update-tests
branch
2 times, most recently
from
August 4, 2023 06:23
859e05f
to
4fd8844
Compare
0xNeshi
changed the title
WIP: Get older tests up-to-date
[WIP] Get older tests up-to-date
Aug 4, 2023
0xNeshi
force-pushed
the
update-tests
branch
7 times, most recently
from
August 11, 2023 07:44
89a0682
to
bec6130
Compare
…act (#267) * Use getChainId in AccountsStrategy * Make AccountsStrategy 'upon strategyInvest reverts when' tests pass with fake contracts * Make AccountsStrategy 'upon strategyInvest and calls the local router' tests pass with fake contracts * Make AccountsStrategy 'upon strategyInvest and calls axelar GMP' tests pass with fake contracts * Make AccountsStrategy 'upon strategyRedeem reverts when' tests pass with fake contracts * Add missing EndowmentRedeemed emissions in strategyRedeem * Make AccountsStrategy 'upon strategyRedeem and calls the local router' tests pass with fake contracts * Make AccountsStrategy 'upon strategyRedeem and calls axelar GMP' tests pass with fake contracts * Move state/facet declaration to root describe * Move ACCOUNT_ID up * Move common fake contracts to root describe * Set type of investRequest to AccountMessages.InvestRequestStruct * Refactor strategy approval state * Use 'const' where applicable * Refactor registrar fakes * Check WITHDRAW_ONLY flow in strategyRedeem * Remove redundant token.transfer.returns(true) * Add missing types * Move lock/liq/gas fee consts up * Move all common fake contracts up * Move 'and calls the local router' outside of 'reverts when' * Make upon strategyRedeemAll work with 'reverts when' and 'and calls the local router' * Rename all constants to upper case * Make upon strategyRedeemAll work with 'and calls axelar GMP' * Add new test case when more than enough balance to pay gas * Reorg. imports * Move 'token.approve.returns' mock to beforeEach * Add missing test cases related to amnt of gas covered by GasFwd * Fix lock. bal calc. in _payForGasWithAccountBalance * Add new test case for _payForGasWithAccountBalance * Move balance setup up * Add revert cases for strategyInvest _payForGasWithAccountBalance * Move strategyInvest > _payForGasWithAccountBalance revert tests to 'and calls axelar GMP' section * Remove redundant check in reverting test for strategyRedeem * Add missing test cases for strategyRedeem * Fix/add test cases for strategyRedeemAll * Add new ext. call tests * Move gasFwd, registrar, router and token fakes init to beforeEach * Add test case for strategyRedeemAll > approvalState == WITHDRAW_ONLY * Remove async func from describes * Reorganize callback tests * Pass 0 tokens to _payForGasWithAccountBalance on redeem funcs * Refactor liqGas calculation Co-authored-by: katzman <[email protected]> * Use liq. percentage to calculate gas to pay from account bal * Revert "Use liq. percentage to calculate gas to pay from account bal" This reverts commit 8e53c23. * Extract investAmt * Add gasPercentFromLiq param to _payForGasWithAccountBalance * Add missing require at least one of locked/liquid is passed * Move comments * Add missing 'Must X at least one of locked/liquid' tests * Use ACCOUNT_ID instead of hardcoded 1s * Add valid value check for gasPercentFromLiq in _payForGasWithAccountBalance * Revert "Add valid value check for gasPercentFromLiq in _payForGasWithAccountBalance" This reverts commit f7e1a0a. * Fix tests * Add more data to InsufficientFundsForGas error * Add more test cases for _execute * Fix wrongful expect awaits into await expects * Fix 'into _execute' tests * Fix all tests * Add missing types to redeemRequests * Fix typo in InsufficientFundsForGas * Fix comments in _payForGasWithAccountBalance * Add missing emit EndowmentRedeemed in _axelarCallbackWithToken * Rename actionStatus to vaultStatus * Rename gasPercentFromLiq to gasRateFromLiq_withPrecision * Use BIG_NUMBA_BASIS instead of PERCENT_BASIS * Add fractional calculation tests for strategy[Invest/Redeem] * Remove extra fields from InsufficientFundsForGas * Merge all strategyRedeem > 'makes all the correct external calls and pays for part of gas fee' tests into one * Fix strategyRedeemAll percentage, changed basis into BIG_NUMBA_BASIS * Add more strategyRedeemAll tests * Add custom descriptions * Add new strategyInvest test cases * Update EndowmentInvested and EndowmentRedeemed events to include endowId * Add missing expects and events to tests * Move state updates in strategyInvest before if/else * Emit custom error on zero amounts set to invest/redeem * Fix _payForGasWithAccountBalance, no longer accepts lock/liq amts * Fix tests * Move ZeroAmount to interface --------- Co-authored-by: katzman <[email protected]>
SovereignAndrey
suggested changes
Aug 15, 2023
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.
Couple things with the IndexFund contract changes I noticed that needs fixing/reverting, but the tests stuff looks good. 👍
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #317
Related #207
Explanation of the solution
Cleaning up only the tests I was certain were intended as "unit tests".
In other places left uses of real dummy contracts, will leave to you @SovereignAndrey @stevieraykatz to decide which ones should be treated as "integration tests" (left as-is) and which as "unit tests" (changed to useWill update all tests where it makes sensesmock
).Instructions on making this work
yarn
oryarn install
to install npm dependenciesyarn test
Note
The PR addresses the "low-hanging fruit" of issues, all tests and their corresponding contracts will be updated one-by-one in follow-up PRs.