test: add AMM slippage error and liquidity withdrawal test cases#634
test: add AMM slippage error and liquidity withdrawal test cases#634nitinraj71 wants to merge 2 commits into
Conversation
Greptile SummaryThis PR adds two new integration test cases to the
Confidence Score: 5/5Safe to merge; the two new tests are structurally correct and the previously reported bugs are all addressed. Both new tests follow sound patterns: the withdraw test captures pre/post balances and asserts LP burn and token receipts; the slippage test correctly moves expect.fail outside the catch block and adds an error-type assertion. No logic errors or incorrect on-chain interactions were found. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "test: change error log matching signatur..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect.fail("The transaction should have failed due to high slippage, but it succeeded."); | ||
| } catch (err: any) { | ||
| expect(err).to.exist; | ||
| } |
There was a problem hiding this comment.
expect.fail() is swallowed by the outer catch, making this test permanently pass
expect.fail(...) throws a Chai AssertionError. That exception is immediately caught by the surrounding catch (err: any) block on line 149, and then expect(err).to.exist evaluates to true — because err is the AssertionError object. This means the test will pass unconditionally: if the swap succeeds (slippage not enforced), expect.fail throws, the catch silently swallows it, and the assertion on line 150 trivially passes. The test provides no real coverage for the slippage guard.
| } catch (err: any) { | ||
| expect(err).to.exist; | ||
| } |
There was a problem hiding this comment.
expect(err).to.exist is always satisfied by any thrown value, including network errors, account-not-found errors, or the AssertionError from expect.fail. The test won't distinguish a real slippage rejection from an unrelated setup failure. Consider asserting the specific program error code (e.g., err.error?.errorCode?.code === "SlippageExceeded") to make sure the right error is being raised.
| const postLiquidity = await connection.getTokenAccountBalance(values.liquidityAccount); | ||
| expect(new BN(postLiquidity.value.amount).toString()).to.equal( | ||
| new BN(initialLiquidity.value.amount).sub(withdrawAmount).toString() | ||
| ); |
There was a problem hiding this comment.
The test only checks that the LP token balance drops by withdrawAmount. It never checks that holderAccountA or holderAccountB balances increased. A regression where the program burns LP tokens but fails to transfer the underlying tokens back would pass this test without notice.
1 participant
Description
This PR expands the integration test coverage for the
swap_exampleprogram within thetoken-swapexample directory. Currently, the test suite only validates the primary "Swap from A to B" happy path.Changes Introduced
Withdraw Liquidity successfullyTest: Validates the end-to-end execution of partial pool share burning, explicit account resolution for theammstate context, and subsequent token balance distribution updates.swap_exact_tokens_for_tokensto mathematically guarantee the program correctly returns an execution error whenmin_output_amountboundaries are breached.Verification
11 passingverified viaanchor testnatively within a clean WSL/Linux sub-system configuration).