test: use "given" keyword in branching trees (#656)

* refactor: using Given keyword in branching Trees (#641) * Using Given keyword on the branching trees (cherry picked from commit d365705) * test: use "it" for test nodes --------- Co-authored-by: Paul Razvan Berg <[email protected]> test: rename to "givenCallerAdmin" test: use given keyword in the integration branching trees test: use given keyword for modifiers for fuzz tests test: use given keyword in test contracts and brancing trees test: refactor test function names to use Given and When test: refactor test function names to use Given and When refactor: reverting in dev comments refactor: rename test function names to cleaner form test: polish branching trees test: improve "mapSymbol" tree test: improve "safeAssetDecimals" tree test: improve "safeAssetSymbol" tree * refactor: givenWithdrawableAmountNotZero modifier name * test: use "when" for time-related branches * test: address feedback in tree wording branch * test: polish branching trees * fix: remove duplicated branch in createWithMilestones.tree --------- Co-authored-by: Shub <[email protected]> Co-authored-by: andreivladbrg <[email protected]> Co-authored-by: Alexander González <[email protected]>
sablier-labs · Aug 23, 2023 · 925630d · 925630d
1 parent 75c76f8
commit 925630d
Show file tree

Hide file tree

Showing 128 changed files with 829 additions and 811 deletions.
diff --git a/test/integration/concrete/comptroller/protocol-fees/protocolFees.t.sol b/test/integration/concrete/comptroller/protocol-fees/protocolFees.t.sol
@@ -19,12 +19,12 @@ contract ProtocolFees_Integration_Concrete_Test is Integration_Test {
         assertEq(actualProtocolFee, expectedProtocolFee, "protocolFees");
     }
 
-    modifier whenProtocolFeeSet() {
+    modifier givenProtocolFeeSet() {
         comptroller.setProtocolFee({ asset: dai, newProtocolFee: defaults.PROTOCOL_FEE() });
         _;
     }
 
-    function test_ProtocolFees() external whenProtocolFeeSet {
+    function test_ProtocolFees() external givenProtocolFeeSet {
         UD60x18 actualProtocolFee = comptroller.protocolFees(dai);
         UD60x18 expectedProtocolFee = defaults.PROTOCOL_FEE();
         assertEq(actualProtocolFee, expectedProtocolFee, "protocolFees");

diff --git a/test/integration/concrete/comptroller/protocol-fees/protocolFees.tree b/test/integration/concrete/comptroller/protocol-fees/protocolFees.tree
@@ -1,5 +1,5 @@
 protocolFees.t.sol
-├── when the protocol fee has not been set
+├── given the protocol fee has not been set
 │  └── it should return zero
-└── when the protocol fee has been set
+└── given the protocol fee has been set
    └── it should return the correct protocol fee
diff --git a/test/integration/concrete/comptroller/toggle-flash-asset/toggleFlashAsset.t.sol b/test/integration/concrete/comptroller/toggle-flash-asset/toggleFlashAsset.t.sol
@@ -33,12 +33,12 @@ contract ToggleFlashAsset_Integration_Concrete_Test is Integration_Test {
         assertTrue(isFlashAsset, "isFlashAsset");
     }
 
-    modifier whenFlagEnabled() {
+    modifier givenFlagEnabled() {
         comptroller.toggleFlashAsset(dai);
         _;
     }
 
-    function test_ToggleFlashAsset() external whenCallerAdmin whenFlagEnabled {
+    function test_ToggleFlashAsset() external whenCallerAdmin givenFlagEnabled {
         // Expect the relevant event to be emitted.
         vm.expectEmit({ emitter: address(comptroller) });
         emit ToggleFlashAsset({ admin: users.admin, asset: dai, newFlag: false });

diff --git a/test/integration/concrete/comptroller/toggle-flash-asset/toggleFlashAsset.tree b/test/integration/concrete/comptroller/toggle-flash-asset/toggleFlashAsset.tree
@@ -2,9 +2,9 @@ toggleFlashAsset.t.sol
 ├── when the caller is not the admin
 │  └── it should revert
 └── when the caller is the admin
-   ├── when the flag is not enabled
+   ├── given the flag is not enabled
    │  ├── it should toggle the flash asset
    │  └── it should emit a {ToggleFlashAsset} event
-   └── when the flag is enabled
+   └── given the flag is enabled
       ├── it should toggle the flash asset
       └── it should emit a {ToggleFlashAsset} event
diff --git a/test/integration/concrete/flash-loan/flash-fee/flashFee.t.sol b/test/integration/concrete/flash-loan/flash-fee/flashFee.t.sol
@@ -8,17 +8,17 @@ import { Errors } from "src/libraries/Errors.sol";
 import { FlashLoan_Integration_Shared_Test } from "../../../shared/flash-loan/FlashLoan.t.sol";
 
 contract FlashFee_Integration_Concrete_Test is FlashLoan_Integration_Shared_Test {
-    function test_RevertWhen_AssetNotFlashLoanable() external {
+    function test_RevertGiven_AssetNotFlashLoanable() external {
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2FlashLoan_AssetNotFlashLoanable.selector, dai));
         flashLoan.flashFee({ asset: address(dai), amount: 0 });
     }
 
-    modifier whenAssetFlashLoanable() {
+    modifier givenAssetFlashLoanable() {
         comptroller.toggleFlashAsset(dai);
         _;
     }
 
-    function test_FlashFee() external whenAssetFlashLoanable {
+    function test_FlashFee() external givenAssetFlashLoanable {
         uint256 amount = 782.23e18;
         uint256 actualFlashFee = flashLoan.flashFee({ asset: address(dai), amount: amount });
         uint256 expectedFlashFee = ud(amount).mul(defaults.FLASH_FEE()).intoUint256();

diff --git a/test/integration/concrete/flash-loan/flash-fee/flashFee.tree b/test/integration/concrete/flash-loan/flash-fee/flashFee.tree
@@ -1,5 +1,5 @@
 flashFee.t.sol
-├── when the asset is not flash loanable
+├── given the asset is not flash loanable
 │  └── it should revert
-└── when the asset is flash loanable
+└── given the asset is flash loanable
    └── it should return the correct flash fee
diff --git a/test/integration/concrete/flash-loan/flash-loan/flashLoan.t.sol b/test/integration/concrete/flash-loan/flash-loan/flashLoan.t.sol
@@ -26,7 +26,7 @@ contract FlashLoanFunction_Integration_Concrete_Test is FlashLoanFunction_Integr
         flashLoan.flashLoan({ receiver: goodFlashLoanReceiver, asset: address(dai), amount: amount, data: bytes("") });
     }
 
-    function test_RevertWhen_AssetNotFlashLoanable() external whenNotDelegateCalled whenAmountNotTooHigh {
+    function test_RevertGiven_AssetNotFlashLoanable() external whenNotDelegateCalled whenAmountNotTooHigh {
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2FlashLoan_AssetNotFlashLoanable.selector, dai));
         flashLoan.flashLoan({ receiver: goodFlashLoanReceiver, asset: address(dai), amount: 0, data: bytes("") });
     }
@@ -35,7 +35,7 @@ contract FlashLoanFunction_Integration_Concrete_Test is FlashLoanFunction_Integr
         external
         whenNotDelegateCalled
         whenAmountNotTooHigh
-        whenAssetFlashLoanable
+        givenAssetFlashLoanable
     {
         // Set the comptroller flash fee so that the calculated fee ends up being greater than 2^128.
         comptroller.setFlashFee({ newFlashFee: ud(1.1e18) });
@@ -54,7 +54,7 @@ contract FlashLoanFunction_Integration_Concrete_Test is FlashLoanFunction_Integr
         external
         whenNotDelegateCalled
         whenAmountNotTooHigh
-        whenAssetFlashLoanable
+        givenAssetFlashLoanable
         whenCalculatedFeeNotTooHigh
     {
         deal({ token: address(dai), to: address(flashLoan), give: LIQUIDITY_AMOUNT });
@@ -71,7 +71,7 @@ contract FlashLoanFunction_Integration_Concrete_Test is FlashLoanFunction_Integr
         external
         whenNotDelegateCalled
         whenAmountNotTooHigh
-        whenAssetFlashLoanable
+        givenAssetFlashLoanable
         whenCalculatedFeeNotTooHigh
         whenBorrowDoesNotFail
     {
@@ -90,7 +90,7 @@ contract FlashLoanFunction_Integration_Concrete_Test is FlashLoanFunction_Integr
         external
         whenNotDelegateCalled
         whenAmountNotTooHigh
-        whenAssetFlashLoanable
+        givenAssetFlashLoanable
         whenCalculatedFeeNotTooHigh
         whenBorrowDoesNotFail
         whenNoReentrancy

diff --git a/test/integration/concrete/flash-loan/flash-loan/flashLoan.tree b/test/integration/concrete/flash-loan/flash-loan/flashLoan.tree
@@ -5,9 +5,9 @@ flashLoan.t.sol
    ├── when the flash loan amount is too high
    │  └── it should revert
    └── when the flash loan amount is not too high
-      ├── when the asset is not flash loanable
+      ├── given the asset is not flash loanable
       │  └── it should revert
-      └── when the asset is flash loanable
+      └── given the asset is flash loanable
          ├── when the calculated fee is too high
          │  └── it should revert
          └── when the calculated fee is not too high
@@ -21,6 +21,6 @@ flashLoan.t.sol
                   │  └── it should revert
                   └── when there is no reentrancy
                      ├── it should execute the flash loan
-                     ├── it should  make the ERC-20 transfers
+                     ├── it should make the ERC-20 transfers
                      ├── it should update the protocol revenues
                      └── it should emit a {FlashLoan} event
diff --git a/test/integration/concrete/flash-loan/max-flash-loan/maxFlashLoan.t.sol b/test/integration/concrete/flash-loan/max-flash-loan/maxFlashLoan.t.sol
@@ -10,12 +10,12 @@ contract MaxFlashLoan_Integration_Concrete_Test is FlashLoan_Integration_Shared_
         assertEq(actualAmount, expectedAmount, "maxFlashLoan amount");
     }
 
-    modifier whenAssetFlashLoanable() {
+    modifier givenAssetFlashLoanable() {
         comptroller.toggleFlashAsset(dai);
         _;
     }
 
-    function test_MaxFlashLoan() external whenAssetFlashLoanable {
+    function test_MaxFlashLoan() external givenAssetFlashLoanable {
         uint256 dealAmount = 14_607_904e18;
         deal({ token: address(dai), to: address(flashLoan), give: dealAmount });
         uint256 actualAmount = flashLoan.maxFlashLoan(address(dai));

diff --git a/test/integration/concrete/flash-loan/max-flash-loan/maxFlashLoan.tree b/test/integration/concrete/flash-loan/max-flash-loan/maxFlashLoan.tree
@@ -1,5 +1,5 @@
 maxFlashLoan.t.sol
-├── when the asset is not flash loanable
+├── given the asset is not flash loanable
 │  └── it should revert
-└── when the asset is flash loanable
+└── given the asset is flash loanable
    └── it should return the correct max flash loan
diff --git a/test/integration/concrete/lockup-dynamic/create-with-milestones/createWithMilestones.t.sol b/test/integration/concrete/lockup-dynamic/create-with-milestones/createWithMilestones.t.sol
@@ -168,7 +168,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         createDefaultStreamWithSegments(segments);
     }
 
-    function test_RevertWhen_EndTimeNotInTheFuture()
+    function test_RevertGiven_EndTimeNotInTheFuture()
         external
         whenNotDelegateCalled
         whenRecipientNonZeroAddress
@@ -225,7 +225,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         lockupDynamic.createWithMilestones(params);
     }
 
-    function test_RevertWhen_ProtocolFeeTooHigh()
+    function test_RevertGiven_ProtocolFeeTooHigh()
         external
         whenNotDelegateCalled
         whenRecipientNonZeroAddress
@@ -264,7 +264,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         whenSegmentMilestonesOrdered
         whenEndTimeInTheFuture
         whenDepositAmountEqualToSegmentAmountsSum
-        whenProtocolFeeNotTooHigh
+        givenProtocolFeeNotTooHigh
     {
         UD60x18 brokerFee = MAX_FEE + ud(1);
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_BrokerFeeTooHigh.selector, brokerFee, MAX_FEE));
@@ -283,7 +283,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         whenSegmentMilestonesOrdered
         whenEndTimeInTheFuture
         whenDepositAmountEqualToSegmentAmountsSum
-        whenProtocolFeeNotTooHigh
+        givenProtocolFeeNotTooHigh
         whenBrokerFeeNotTooHigh
     {
         address nonContract = address(8128);
@@ -311,7 +311,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         whenSegmentMilestonesOrdered
         whenEndTimeInTheFuture
         whenDepositAmountEqualToSegmentAmountsSum
-        whenProtocolFeeNotTooHigh
+        givenProtocolFeeNotTooHigh
         whenBrokerFeeNotTooHigh
         whenAssetContract
     {
@@ -330,7 +330,7 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
         whenSegmentMilestonesOrdered
         whenEndTimeInTheFuture
         whenDepositAmountEqualToSegmentAmountsSum
-        whenProtocolFeeNotTooHigh
+        givenProtocolFeeNotTooHigh
         whenBrokerFeeNotTooHigh
         whenAssetContract
         whenAssetERC20

diff --git a/test/integration/concrete/lockup-dynamic/create-with-milestones/createWithMilestones.tree b/test/integration/concrete/lockup-dynamic/create-with-milestones/createWithMilestones.tree
@@ -31,14 +31,12 @@ createWithMilestones.t.sol
                            ├── when the deposit amount is not equal to the segment amounts sum
                            │  └── it should revert
                            └── when the deposit amount is equal to the segment amounts sum
-                              ├── when the protocol fee is too high
+                              ├── given the protocol fee is too high
                               │  └── it should revert
-                              └── when the protocol fee is not too high
+                              └── given the protocol fee is not too high
                                  ├── when the broker fee is too high
                                  │  └── it should revert
                                  └── when the broker fee is not too high
-                                    ├── when the asset is not a contract
-                                    │  └── it should revert
                                     ├── when the asset is not a contract
                                     │  └── it should revert
                                     └── when the asset is a contract

diff --git a/test/integration/concrete/lockup-dynamic/get-range/getRange.t.sol b/test/integration/concrete/lockup-dynamic/get-range/getRange.t.sol
@@ -7,17 +7,17 @@ import { LockupDynamic } from "src/types/DataTypes.sol";
 import { LockupDynamic_Integration_Concrete_Test } from "../LockupDynamic.t.sol";
 
 contract GetRange_LockupDynamic_Integration_Concrete_Test is LockupDynamic_Integration_Concrete_Test {
-    function test_RevertWhen_Null() external {
+    function test_RevertGiven_Null() external {
         uint256 nullStreamId = 1729;
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId));
         lockupDynamic.getRange(nullStreamId);
     }
 
-    modifier whenNotNull() {
+    modifier givenNotNull() {
         _;
     }
 
-    function test_GetRange() external whenNotNull {
+    function test_GetRange() external givenNotNull {
         uint256 streamId = createDefaultStream();
         LockupDynamic.Range memory actualRange = lockupDynamic.getRange(streamId);
         LockupDynamic.Range memory expectedRange = defaults.lockupDynamicRange();

diff --git a/test/integration/concrete/lockup-dynamic/get-range/getRange.tree b/test/integration/concrete/lockup-dynamic/get-range/getRange.tree
@@ -1,5 +1,5 @@
 getRange.t.sol
-├── when the id references a null stream
+├── given the id references a null stream
 │  └── it should revert
-└── when the id does not reference a null stream
+└── given the id does not reference a null stream
    └── it should return the correct range
diff --git a/test/integration/concrete/lockup-dynamic/get-segments/getSegments.t.sol b/test/integration/concrete/lockup-dynamic/get-segments/getSegments.t.sol
@@ -7,17 +7,17 @@ import { LockupDynamic } from "src/types/DataTypes.sol";
 import { LockupDynamic_Integration_Concrete_Test } from "../LockupDynamic.t.sol";
 
 contract GetSegments_LockupDynamic_Integration_Concrete_Test is LockupDynamic_Integration_Concrete_Test {
-    function test_RevertWhen_Null() external {
+    function test_RevertGiven_Null() external {
         uint256 nullStreamId = 1729;
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId));
         lockupDynamic.getSegments(nullStreamId);
     }
 
-    modifier whenNotNull() {
+    modifier givenNotNull() {
         _;
     }
 
-    function test_GetSegments() external whenNotNull {
+    function test_GetSegments() external givenNotNull {
         uint256 streamId = createDefaultStream();
         LockupDynamic.Segment[] memory actualSegments = lockupDynamic.getSegments(streamId);
         LockupDynamic.Segment[] memory expectedSegments = defaults.segments();

diff --git a/test/integration/concrete/lockup-dynamic/get-segments/getSegments.tree b/test/integration/concrete/lockup-dynamic/get-segments/getSegments.tree
@@ -1,5 +1,5 @@
 getSegments.t.sol
-├── when the id references a null stream
+├── given the id references a null stream
 │  └── it should revert
-└── when the id does not reference a null stream
+└── given the id does not reference a null stream
    └── it should return the correct segments
diff --git a/test/integration/concrete/lockup-dynamic/get-stream/getStream.t.sol b/test/integration/concrete/lockup-dynamic/get-stream/getStream.t.sol
@@ -14,29 +14,29 @@ contract GetStream_LockupDynamic_Integration_Concrete_Test is LockupDynamic_Inte
         defaultStreamId = createDefaultStream();
     }
 
-    function test_RevertWhen_Null() external {
+    function test_RevertGiven_Null() external {
         uint256 nullStreamId = 1729;
         vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId));
         lockupDynamic.getStream(nullStreamId);
     }
 
-    modifier whenNotNull() {
+    modifier givenNotNull() {
         _;
     }
 
-    function test_GetStream_StatusSettled() external whenNotNull {
+    function test_GetStream_StatusSettled() external givenNotNull {
         vm.warp({ timestamp: defaults.END_TIME() });
         LockupDynamic.Stream memory actualStream = lockupDynamic.getStream(defaultStreamId);
         LockupDynamic.Stream memory expectedStream = defaults.lockupDynamicStream();
         expectedStream.isCancelable = false;
         assertEq(actualStream, expectedStream);
     }
 
-    modifier whenStatusNotSettled() {
+    modifier givenStatusNotSettled() {
         _;
     }
 
-    function test_GetStream() external whenNotNull whenStatusNotSettled {
+    function test_GetStream() external givenNotNull givenStatusNotSettled {
         uint256 streamId = createDefaultStream();
         LockupDynamic.Stream memory actualStream = lockupDynamic.getStream(streamId);
         LockupDynamic.Stream memory expectedStream = defaults.lockupDynamicStream();

diff --git a/test/integration/concrete/lockup-dynamic/get-stream/getStream.tree b/test/integration/concrete/lockup-dynamic/get-stream/getStream.tree
@@ -1,8 +1,8 @@
 getStream.t.sol
-├── when the id references a null stream
+├── given the id references a null stream
 │  └── it should revert
-└── when the id does not reference a null stream
-   ├── when the stream is settled
+└── given the id does not reference a null stream
+   ├── given the stream is settled
    │  └── it should adjust the `isCancelable` flag and return the stream
-   └── when the id does not reference a null stream
+   └── given the stream is not settled
       └── it should return the stream