Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: always test external methods #168

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions test/unit/BCoWFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,10 @@ contract BCoWFactoryTest is Test {
assertEq(_newFactory.getBDao(), _bDao);
}

function test__newBPoolWhenCalled() external {
function test_NewBPoolWhenCalled() external {
IBCoWPool _newPool = IBCoWPool(address(factory.newBPool()));
vm.prank(address(factory));
bytes memory _expectedCode = address(new BCoWPool(solutionSettler, appData)).code; // NOTE: uses nonce 1
address _futurePool = vm.computeCreateAddress(address(factory), 2);

IBCoWPool _newPool = IBCoWPool(address(factory.call__newBPool()));
assertEq(address(_newPool), _futurePool);
bytes memory _expectedCode = address(new BCoWPool(solutionSettler, appData)).code;
// it should set the new BCoWPool solution settler
assertEq(address(_newPool.SOLUTION_SETTLER()), solutionSettler);
// it should set the new BCoWPool app data
Expand Down
2 changes: 1 addition & 1 deletion test/unit/BCoWFactory.tree
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ BCoWFactoryTest::constructor
├── it should set app data
└── it should set BDao

BCoWFactoryTest::_newBPool
BCoWFactoryTest::newBPool
└── when called
├── it should set BCoWPool solution settler
├── it should set BCoWPool app data
Expand Down
22 changes: 7 additions & 15 deletions test/unit/BFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ contract BFactoryTest is Test {
assertEq(newFactory.getBDao(), _bDao);
}

function test_NewBPoolWhenCalled(address _deployer, address _newBPool) external {
assumeNotForgeAddress(_newBPool);
vm.mockCall(_newBPool, abi.encodePacked(IBPool.setController.selector), abi.encode());
factory.mock_call__newBPool(IBPool(_newBPool));
function test_NewBPoolWhenCalled(address _deployer) external {
address _newBPool = vm.computeCreateAddress(address(factory), 1);
// it should call _newBPool
factory.expectCall__newBPool();
// it should set the controller of the newBPool to the caller
Expand All @@ -42,24 +40,18 @@ contract BFactoryTest is Test {

vm.prank(_deployer);
IBPool pool = factory.newBPool();
vm.prank(address(factory)); // bpool has immutable FACTORY
bytes memory expectedCode = address(new BPool()).code;

// it should deploy a new BPool
assertEq(address(pool).code, expectedCode);

// it should add the newBPool to the list of pools
assertTrue(factory.isBPool(address(_newBPool)));
// it should return the address of the new BPool
assertEq(address(pool), _newBPool);
}

function test__newBPoolWhenCalled() external {
vm.prank(address(factory));
bytes memory _expectedCode = address(new BPool()).code; // NOTE: uses nonce 1
address _futurePool = vm.computeCreateAddress(address(factory), 2);

address _newBPool = address(factory.call__newBPool());
assertEq(_newBPool, _futurePool);
// it should deploy a new BPool
assertEq(_newBPool.code, _expectedCode);
}

function test_SetBDaoRevertWhen_TheSenderIsNotTheCurrentBDao(address _caller) external {
vm.assume(_caller != factoryDeployer);

Expand Down
5 changes: 1 addition & 4 deletions test/unit/BFactory.tree
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,12 @@ BFactoryTest::constructor
BFactoryTest::newBPool
└── when called
├── it should call _newBPool
├── it should deploy a new BPool
├── it should add the newBPool to the mapping of pools
├── it should return the address of the new BPool
├── it should emit a PoolCreated event
└── it should set the controller of the new BPool to the caller

BFactoryTest::_newBPool
└── when called
└── it should deploy a new BPool

Comment on lines 5 to -16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't follow why we're doing this, isn't it better to test that the internal gets called and test the internal separately?

BFactoryTest::setBDao
├── when the sender is not the current BDao
│ └── it should revert
Expand Down
Loading