test: always call the top-level methods instead of internal ones

defi-wonderland · Jul 18, 2024 · 7fb8c8e · 7fb8c8e
1 parent 598b6ee
commit 7fb8c8e
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 26 deletions.
diff --git a/test/unit/BCoWFactory.t.sol b/test/unit/BCoWFactory.t.sol
@@ -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

diff --git a/test/unit/BCoWFactory.tree b/test/unit/BCoWFactory.tree
@@ -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

diff --git a/test/unit/BFactory.t.sol b/test/unit/BFactory.t.sol
@@ -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
@@ -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);
 

diff --git a/test/unit/BFactory.tree b/test/unit/BFactory.tree
@@ -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
-
 BFactoryTest::setBDao
 ├── when the sender is not the current BDao
 │   └── it should revert