Cyfrin / foundry-defi-stablecoin-cu

241 stars 107 forks source link

Improved test case for testRevertsIfTransferFromFails #83

Open a1111198 opened 2 months ago

a1111198 commented 2 months ago

This PR updates the test case testRevertsIfTransferFromFails to improve clarity and avoid confusion regarding the endogenous vs. exogenous nature of the contracts involved.

Changes Made:

Diff:

// Arrange - Setup
address owner = msg.sender;
vm.prank(owner);
-MockFailedTransferFrom mockDsc = new MockFailedTransferFrom();
-tokenAddresses = [address(mockDsc)];
+MockFailedTransferFrom mockWethTransferFailed = new MockFailedTransferFrom();
+tokenAddresses = [address(mockWethTransferFailed)];
feedAddresses = [ethUsdPriceFeed];
vm.prank(owner);
-DSCEngine mockDsce = new DSCEngine(tokenAddresses, feedAddresses, address(mockDsc));
-mockDsc.mint(user, amountCollateral);
+DSCEngine mockDsce = new DSCEngine(tokenAddresses, feedAddresses, address(dsc));
+mockWethTransferFailed.mint(user, amountCollateral);

// Arrange - User
vm.startPrank(user);
-ERC20Mock(address(mockDsc)).approve(address(mockDsce), amountCollateral);
+ERC20Mock(address(mockWethTransferFailed)).approve(address(mockDsce), amountCollateral);
// Act / Assert
vm.expectRevert(DSCEngine.DSCEngine__TransferFailed.selector);
-mockDsce.depositCollateral(address(mockDsc), amountCollateral);
+mockDsce.depositCollateral(address(mockWethTransferFailed), amountCollateral);
vm.stopPrank();

By making these changes, the test case now more clearly represents an exogenous collateral scenario, reducing potential confusion.

PatrickAlphaC commented 2 months ago

Could you change it so the formatting isn't different? You should be able to run forge fmt to get the same formatting as me.

a1111198 commented 2 months ago

I've run forge fmtto standardize the formatting and committed the changes