From c7cc86474786fe5a5f3c46e374fd201471344701 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 22 Apr 2024 16:34:47 +0300 Subject: [PATCH] docs: improve readability for streamId requirements test: say "given" for balance zero tests --- src/interfaces/ISablierV2OpenEnded.sol | 15 +++++---------- .../withdraw-multiple/withdrawMultiple.t.sol | 1 + .../withdraw-multiple/withdrawMultiple.tree | 6 +++--- test/integration/withdraw/withdraw.t.sol | 6 +++++- test/integration/withdraw/withdraw.tree | 4 ++-- test/utils/Modifiers.sol | 6 +++++- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 4579078e..84538ad1 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -152,8 +152,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requiremenets: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// - `newRatePerSecond` must be greater than zero and not equal to the current rate per second. /// @@ -167,8 +166,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// /// @param streamId The ID of the stream to cancel. @@ -289,8 +287,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `depositAmount` must be greater than zero. /// /// @param streamId The ID of the stream to deposit on. @@ -315,8 +312,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the sender. /// - `refundAmount` must be greater than zero and must not exceed the refundable amount. /// @@ -330,8 +326,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - // - `streamId` must not reference a null stream. - /// - `streamId` must reference a canceled stream. + // - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// - `ratePerSecond` must be greater than zero. /// diff --git a/test/integration/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/withdraw-multiple/withdrawMultiple.t.sol index 7459020f..957e2f24 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/integration/withdraw-multiple/withdrawMultiple.t.sol @@ -220,6 +220,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test { givenNotCanceled whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { defaultDeposit(); defaultDeposit(defaultStreamIds[1]); diff --git a/test/integration/withdraw-multiple/withdrawMultiple.tree b/test/integration/withdraw-multiple/withdrawMultiple.tree index 5e60a542..c1fccd8a 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.tree +++ b/test/integration/withdraw-multiple/withdrawMultiple.tree @@ -28,11 +28,11 @@ withdrawMultiple.t.sol ├── when some withdrawal times are in the future │ └── it should revert └── when none withdrawal times are in the future - ├── when all balances are zero + ├── given all balances are zero │ └── it should revert - ├── when some balances are zero + ├── given some balances are zero │ └── it should revert - └── when all balances are greater than zero + └── given all balances are greater than zero ├── it should make the withdrawals ├── it should update the times └── it should emit multiple {WithdrawFromOpenEndedStream} events \ No newline at end of file diff --git a/test/integration/withdraw/withdraw.t.sol b/test/integration/withdraw/withdraw.t.sol index 91a92bdf..842d1d68 100644 --- a/test/integration/withdraw/withdraw.t.sol +++ b/test/integration/withdraw/withdraw.t.sol @@ -120,7 +120,7 @@ contract Withdraw_Integration_Test is Integration_Test { openEnded.withdraw({ streamId: defaultStreamId, to: users.recipient, time: futureTime }); } - function test_RevertWhen_BalanceZero() + function test_RevertGiven_BalanceZero() external whenNotDelegateCalled givenNotNull @@ -147,6 +147,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { openEnded.withdraw({ streamId: defaultStreamId, to: users.recipient, time: WITHDRAW_TIME }); @@ -168,6 +169,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { address unknownCaller = address(0xCAFE); resetPrank({ msgSender: unknownCaller }); @@ -192,6 +194,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero whenCallerRecipient { // Set the timestamp to 1 month ago to create the stream with the same `lastTimeUpdate` as `defaultStreamId`. @@ -212,6 +215,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero whenCallerRecipient { _test_Withdraw(defaultStreamId, dai); diff --git a/test/integration/withdraw/withdraw.tree b/test/integration/withdraw/withdraw.tree index 4a262b41..019148d6 100644 --- a/test/integration/withdraw/withdraw.tree +++ b/test/integration/withdraw/withdraw.tree @@ -23,9 +23,9 @@ withdraw.t.sol ├── when the withdrawal time is in the future │ └── it should revert └── when the withdrawal time is not in the future - ├── when the balance is zero + ├── given the balance is zero │ └── it should revert - └── when the balance is not zero + └── given the balance is not zero ├── when the caller is not the recipient │ ├── when the caller is the sender │ │ └── it should make the withdrawal diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index ea2230fb..b2e5525c 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -94,7 +94,7 @@ abstract contract Modifiers { WITHDRAW //////////////////////////////////////////////////////////////////////////*/ - modifier whenToNonZeroAddress() { + modifier givenBalanceNotZero() { _; } @@ -102,6 +102,10 @@ abstract contract Modifiers { _; } + modifier whenToNonZeroAddress() { + _; + } + modifier whenWithdrawalAddressIsRecipient() { _; }