Skip to content

Commit

Permalink
test(refactor): periphery tests to use Bulloak (#1025)
Browse files Browse the repository at this point in the history
* test(refactor): periphery tests to use Bulloak

test(tree): dont use is in tree

* test(tree): remove "does" word

* test(tree): use shorter form in create factory

---------

Co-authored-by: andreivladbrg <andreivladbrg@gmail.com>
  • Loading branch information
smol-ninja and andreivladbrg committed Sep 11, 2024
1 parent 8b974d4 commit 0da540c
Show file tree
Hide file tree
Showing 52 changed files with 259 additions and 323 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
uses: "sablier-labs/reusable-workflows/.github/workflows/bulloak-check.yml@main"
with:
skip-modifiers: true
tree-path: "test/core"
tree-path: "test"

build:
uses: "sablier-labs/reusable-workflows/.github/workflows/forge-build.yml@main"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract TokenURI_LockupDynamic_Integration_Concrete_Test is LockupDynamic_Integ
}
}

function test_RevertGiven_NFTDoesNotExist() external {
function test_RevertGiven_NFTNotExist() external {
uint256 nullStreamId = 1729;
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, nullStreamId));
lockupDynamic.tokenURI({ tokenId: nullStreamId });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
TokenURI_LockupDynamic_Integration_Concrete_Test
├── given NFT does not exist
├── given NFT not exist
│ └── it should revert
└── given NFT exists
├── when token URI decoded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract TokenURI_LockupLinear_Integration_Concrete_Test is LockupLinear_Integra
}
}

function test_RevertGiven_NFTDoesNotExist() external {
function test_RevertGiven_NFTNotExist() external {
uint256 nullStreamId = 1729;
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, nullStreamId));
lockupLinear.tokenURI({ tokenId: nullStreamId });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
TokenURI_LockupLinear_Integration_Concrete_Test
├── given NFT does not exist
├── given NFT not exist
│ └── it should revert
└── given NFT exists
├── when token URI decoded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ CreateWithTimestamps_LockupTranched_Integration_Concrete_Test
└── when timestamps strictly increasing
├── when deposit amount not equal tranche amounts sum
│ └── it should revert
└── when the deposit amount equals tranche amounts sum
└── when deposit amount equals tranche amounts sum
├── when broker fee exceeds max value
│ └── it should revert
└── when broker fee not exceed max value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract TokenURI_LockupTranched_Integration_Concrete_Test is LockupTranched_Int
}
}

function test_RevertGiven_NFTDoesNotExist() external {
function test_RevertGiven_NFTNotExist() external {
uint256 nullStreamId = 1729;
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, nullStreamId));
lockupTranched.tokenURI({ tokenId: nullStreamId });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
TokenURI_LockupTranched_Integration_Concrete_Test
├── given NFT does not exist
├── given NFT not exist
│ └── it should revert
└── given NFT exists
├── when token URI decoded
Expand Down
2 changes: 1 addition & 1 deletion test/core/integration/concrete/lockup/burn/burn.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ abstract contract Burn_Integration_Concrete_Test is Integration_Test, Lockup_Int
_;
}

function test_RevertGiven_NFTDoesNotExist()
function test_RevertGiven_NFTNotExist()
external
whenNoDelegateCall
givenNotNull
Expand Down
2 changes: 1 addition & 1 deletion test/core/integration/concrete/lockup/burn/burn.tree
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Burn_Integration_Concrete_Test
├── when unauthorized caller
│ └── it should revert
└── when authorized caller
├── given NFT does not exist
├── given NFT not exist
│ └── it should revert
└── given NFT exists
├── given non transferable NFT
Expand Down
22 changes: 11 additions & 11 deletions test/core/integration/concrete/lockup/withdraw/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressNotRecipient(false)
{
// Simulate the passage of time.
Expand All @@ -136,7 +136,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressNotRecipient(true)
{
// Simulate the passage of time.
Expand Down Expand Up @@ -169,7 +169,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
{
// Make the unknown address the caller in this test.
Expand All @@ -194,7 +194,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
{
// Simulate the passage of time.
Expand All @@ -216,7 +216,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
{
Expand Down Expand Up @@ -248,7 +248,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down Expand Up @@ -296,7 +296,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down Expand Up @@ -332,7 +332,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down Expand Up @@ -362,7 +362,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down Expand Up @@ -395,7 +395,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down Expand Up @@ -445,7 +445,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenWithdrawalAddressRecipient
whenCallerSender
givenEndTimeInFuture
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Withdraw_Integration_Concrete_Test
└── when non zero withdraw amount
├── when withdraw amount overdraws
│ └── it should revert
└── when withdraw amount does not overdraw
└── when withdraw amount not overdraw
├── when withdrawal address not recipient
│ ├── when caller not approved third party or recipient
│ │ └── it should revert
Expand Down
2 changes: 1 addition & 1 deletion test/core/integration/fuzz/lockup-dynamic/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract Withdraw_LockupDynamic_Integration_Fuzz_Test is
givenNotNull
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
{
vm.assume(params.segments.length != 0);
vm.assume(params.to != address(0));
Expand Down
2 changes: 1 addition & 1 deletion test/core/integration/fuzz/lockup-tranched/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract Withdraw_LockupTranched_Integration_Fuzz_Test is
givenNotNull
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
{
vm.assume(params.tranches.length != 0);
vm.assume(params.to != address(0));
Expand Down
8 changes: 4 additions & 4 deletions test/core/integration/fuzz/lockup/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
givenNotNull
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
{
vm.assume(caller != users.sender && caller != users.recipient);

Expand Down Expand Up @@ -58,7 +58,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
givenNotDEPLETEDStatus
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
{
vm.assume(to != address(0));

Expand Down Expand Up @@ -100,7 +100,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
givenNotNull
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
whenCallerRecipient
{
timeJump = _bound(timeJump, defaults.CLIFF_DURATION(), defaults.TOTAL_DURATION() - 1 seconds);
Expand Down Expand Up @@ -167,7 +167,7 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I
givenNotNull
whenWithdrawalAddressNotZero
whenNonZeroWithdrawAmount
whenWithdrawAmountDoesNotOverdraw
whenWithdrawAmountNotOverdraw
givenNotCanceledStream
{
timeJump = _bound(timeJump, defaults.CLIFF_DURATION(), defaults.TOTAL_DURATION() * 2);
Expand Down
2 changes: 1 addition & 1 deletion test/core/integration/shared/lockup/withdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ abstract contract Withdraw_Integration_Shared_Test is Lockup_Integration_Shared_
_;
}

modifier whenWithdrawAmountDoesNotOverdraw() {
modifier whenWithdrawAmountNotOverdraw() {
_;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithDurationsLD_Integration_Test is Periphery_Test {
batchLockup.createWithDurationsLD(lockupDynamic, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithDurations() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithDurationsLD.t.sol
├── when the batch size is zero
CreateWithDurationsLD_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with durations
└── it should perform the ERC-20 transfers
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithDurationsLL_Integration_Test is Periphery_Test {
batchLockup.createWithDurationsLL(lockupLinear, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithDurations() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithDurationsLL.t.sol
├── when the batch size is zero
CreateWithDurationsLL_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with durations
└── it should perform the ERC-20 transfers
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithDurationsLT_Integration_Test is Periphery_Test {
batchLockup.createWithDurationsLT(lockupTranched, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithDurations() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithDurationsLT.t.sol
├── when the batch size is zero
CreateWithDurationsLT_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with durations
└── it should perform the ERC-20 transfers
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithTimestampsLD_Integration_Test is Periphery_Test {
batchLockup.createWithTimestampsLD(lockupDynamic, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithTimestamps() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithTimestampsLD.t.sol
├── when the batch size is zero
CreateWithTimestampsLD_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with timestamps
└── it should perform the ERC-20 transfers
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithTimestampsLL_Integration_Test is Periphery_Test {
batchLockup.createWithTimestampsLL(lockupLinear, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithTimestamps() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithTimestampsLL.t.sol
├── when the batch size is zero
CreateWithTimestampsLL_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with timestamps
└── it should perform the ERC-20 transfers
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ contract CreateWithTimestampsLT_Integration_Test is Periphery_Test {
batchLockup.createWithTimestampsLT(lockupTranched, dai, batchParams);
}

modifier whenBatchSizeNotZero() {
_;
}

function test_BatchCreateWithTimestamps() external whenBatchSizeNotZero {
function test_WhenBatchSizeNotZero() external {
// Asset flow: Sender → batchLockup → SablierLockup
// Expect transfers from Alice to the batchLockup, and then from the batchLockup to the Lockup contract.
expectCallToTransferFrom({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
createWithTimestampsLT.t.sol
├── when the batch size is zero
CreateWithTimestampsLT_Integration_Test
├── when batch size zero
│ └── it should revert
└── when the batch size is not zero
└── when batch size not zero
├── it should create a batch of streams with timestamps
└── it should perform the ERC-20 transfers
Loading

0 comments on commit 0da540c

Please sign in to comment.