test: polish Iaro's changes

test/Base.t.sol

@@ -127,7 +127,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, StdUtils {
     }
 
     /// @dev Generates an address by hashing the name, labels the address and funds it with test assets.
-    function createUser(string memory name) private returns (address payable addr) {
+    function createUser(string memory name) internal returns (address payable addr) {
         addr = payable(makeAddr(name));
         vm.deal({ account: addr, newBalance: 100 ether });
     }
@@ -138,7 +138,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, StdUtils {
     }
 
     /// @dev Conditionally deploy the registry either normally or from a source precompiled with `--via-ir`.
-    function deployRegistryConditionally() private {
+    function deployRegistryConditionally() internal {
         if (!isTestOptimizedProfile()) {
             registry = new PRBProxyRegistry();
         } else {
@@ -149,7 +149,7 @@ abstract contract Base_Test is Assertions, Events, StdCheats, StdUtils {
     }
 
     /// @dev Reads the proxy bytecode either normally or from precompiled source.
-    function getProxyBytecode() private returns (bytes memory) {
+    function getProxyBytecode() internal returns (bytes memory) {
         if (!isTestOptimizedProfile()) {
             return type(PRBProxy).creationCode;
         } else {

test/mocks/plugins/PluginReverter.sol

@@ -13,7 +13,7 @@ contract PluginReverter is IPRBProxyPlugin, TargetReverter {
         methods[1] = this.withCustomError.selector;
         methods[2] = this.withRequire.selector;
         methods[3] = this.withReasonString.selector;
-        methods[4] = this.dueToNoPayableFunction.selector;
+        methods[4] = this.notPayable.selector;
 
         return methods;
     }

test/mocks/targets/TargetReverter.sol

@@ -20,7 +20,7 @@ contract TargetReverter {
         revert("You shall not pass");
     }
 
-    function dueToNoPayableFunction() external pure returns (uint256) {
+    function notPayable() external pure returns (uint256) {
         return 0;
     }
 }

test/proxy/execute/execute.t.sol

@@ -143,13 +143,13 @@ contract Execute_Test is Proxy_Test {
         proxy.execute(address(targets.reverter), data);
     }
 
-    function test_RevertWhen_Error_NoPayableFunction()
+    function test_RevertWhen_Error_NotPayable()
         external
         whenCallerAuthorized
         whenTargetContract
         whenDelegateCallReverts
     {
-        bytes memory data = bytes.concat(targets.reverter.dueToNoPayableFunction.selector);
+        bytes memory data = bytes.concat(targets.reverter.notPayable.selector);
         vm.expectRevert();
         proxy.execute{ value: 0.1 ether }(address(targets.reverter), data);
     }

test/proxy/execute/execute.tree

@@ -22,11 +22,11 @@ execute.t.sol
          │     ├── it should revert with a custom error
          │     ├── it should revert with a require
          │     ├── it should revert with a reason string
-         │     └── it should revert due to no payable function
+         │     └── it should revert when sending ETH to a non-payable function
          └── when the delegate call does not revert
-            ├── when Ether is sent
+            ├── when ETH is sent
             │  └── it should return the Ether amount
-            └── when no Ether is sent
+            └── when no ETH is sent
                ├── when the target self-destructs
                │  └── it should return an empty response and send the ETH to the SELFDESTRUCT recipient
                └── when the target does not self-destruct

test/proxy/receive/receive.t.sol

@@ -4,7 +4,7 @@ pragma solidity >=0.8.19 <=0.9.0;
 import { Proxy_Test } from "../Proxy.t.sol";
 
 contract Receive_Test is Proxy_Test {
-    uint256 private value = 1 ether;
+    uint256 internal value = 1 ether;
 
     function setUp() public virtual override {
         Proxy_Test.setUp();

test/proxy/run-plugin/runPlugin.t.sol

@@ -128,8 +128,8 @@ contract RunPlugin_Test is Proxy_Test {
         registry.installPlugin(plugins.selfDestructer);
         (bool success, bytes memory response) =
             address(proxy).call(abi.encodeWithSelector(plugins.selfDestructer.destroyMe.selector, users.bob));
-        assertTrue(success);
-        assertEq(response.length, 0);
+        assertTrue(success, "selfDestructer.destroyMe failed");
+        assertEq(response.length, 0, "selfDestructer.destroyMe response length mismatch");
 
         // Assert that Bob's balance has increased by the contract's balance.
         uint256 actualBobBalance = users.bob.balance;
@@ -148,6 +148,22 @@ contract RunPlugin_Test is Proxy_Test {
         whenDelegateCallDoesNotRevert
         whenNoEtherSent
         whenPluginDoesNotSelfDestruct
+    {
+        registry.installPlugin(plugins.basic);
+        (bool success, bytes memory actualResponse) =
+            address(proxy).call(abi.encodeWithSelector(plugins.basic.foo.selector));
+        bytes memory expectedResponse = abi.encode(bytes("foo"));
+        assertTrue(success, "basic.foo failed");
+        assertEq(actualResponse, expectedResponse, "basic.foo response mismatch");
+    }
+
+    function test_RunPlugin_Event()
+        external
+        whenPluginInstalled
+        whenDelegateCallReverts
+        whenDelegateCallDoesNotRevert
+        whenNoEtherSent
+        whenPluginDoesNotSelfDestruct
     {
         registry.installPlugin(plugins.basic);
         vm.expectEmit({ emitter: address(proxy) });
@@ -156,10 +172,7 @@ contract RunPlugin_Test is Proxy_Test {
             data: abi.encodeWithSelector(TargetBasic.foo.selector),
             response: abi.encode(bytes("foo"))
         });
-        (bool success, bytes memory actualResponse) =
-            address(proxy).call(abi.encodeWithSelector(plugins.basic.foo.selector));
-        assertTrue(success);
-        bytes memory expectedResponse = abi.encode(bytes("foo"));
-        assertEq(actualResponse, expectedResponse, "basic.foo response mismatch");
+        (bool success,) = address(proxy).call(abi.encodeWithSelector(plugins.basic.foo.selector));
+        success;
     }
 }

test/proxy/run-plugin/runPlugin.tree

@@ -17,11 +17,12 @@ runPlugin.t.sol
       │     ├── it should revert with a require
       │     └── it should revert with a reason string
       └── when the delegate call does not revert
-         ├── when Ether is sent
-         │  └── it should return the Ether amount
-         └── when no Ether is sent
+         ├── when the plugin receives ETH
+         │  └── it should return the ETH amount
+         └── when the plugin does not receive ETH
             ├── when the plugin self-destructs
             │  └── it should return an empty response and send the ETH to the SELFDESTRUCT recipient
             └── when the plugin does not self-destruct
-               └── it should run the plugin and emit a {RunPlugin} event
+               ├── it should run the plugin
+               └── it should emit a {RunPlugin} event
 

test/registry/install-plugin/installPlugin.t.sol

@@ -9,8 +9,9 @@ import { Registry_Test } from "../Registry.t.sol";
 contract InstallPlugin_Test is Registry_Test {
     function test_RevertWhen_CallerDoesNotHaveProxy() external {
         vm.expectRevert(
-            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.alice)
+            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.bob)
         );
+        changePrank({ msgSender: users.bob });
         registry.installPlugin(plugins.empty);
     }
 

test/registry/set-permission/setPermission.t.sol

@@ -8,8 +8,9 @@ import { Registry_Test } from "../Registry.t.sol";
 contract SetPermission_Test is Registry_Test {
     function test_RevertWhen_CallerDoesNotHaveProxy() external {
         vm.expectRevert(
-            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.alice)
+            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.bob)
         );
+        changePrank({ msgSender: users.bob });
         registry.setPermission({ envoy: users.envoy, target: address(targets.basic), permission: true });
     }
 

test/registry/uninstall-plugin/uninstallPlugin.t.sol

@@ -9,8 +9,9 @@ import { Registry_Test } from "../Registry.t.sol";
 contract UninstallPlugin_Test is Registry_Test {
     function test_RevertWhen_CallerDoesNotHaveProxy() external {
         vm.expectRevert(
-            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.alice)
+            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_CallerDoesNotHaveProxy.selector, users.bob)
         );
+        changePrank({ msgSender: users.bob });
         registry.uninstallPlugin(plugins.empty);
     }
 
@@ -20,13 +21,10 @@ contract UninstallPlugin_Test is Registry_Test {
     }
 
     function test_RevertWhen_PluginUnknown() external whenCallerHasProxy {
-        // Assert that every plugin method is uninstalled.
-        checkThatPluginMethodsArentInstalled(users.alice, plugins.basic);
-
         vm.expectRevert(
-            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_PluginUnknown.selector, plugins.basic)
+            abi.encodeWithSelector(IPRBProxyRegistry.PRBProxyRegistry_PluginUnknown.selector, plugins.empty)
         );
-        registry.uninstallPlugin(plugins.basic);
+        registry.uninstallPlugin(plugins.empty);
     }
 
     modifier whenPluginKnown() {
@@ -39,15 +37,11 @@ contract UninstallPlugin_Test is Registry_Test {
         registry.uninstallPlugin(plugins.basic);
 
         // Assert that every plugin method has been uninstalled.
-        checkThatPluginMethodsArentInstalled(users.alice, plugins.basic);
-    }
-
-    function checkThatPluginMethodsArentInstalled(address owner, IPRBProxyPlugin plugin) private {
-        bytes4[] memory pluginMethods = plugin.getMethods();
+        bytes4[] memory pluginMethods = plugins.basic.getMethods();
         for (uint256 i = 0; i < pluginMethods.length; ++i) {
-            IPRBProxyPlugin actualPlugin = registry.getPluginByOwner({ owner: owner, method: pluginMethods[i] });
+            IPRBProxyPlugin actualPlugin = registry.getPluginByOwner({ owner: users.alice, method: pluginMethods[i] });
             IPRBProxyPlugin expectedPlugin = IPRBProxyPlugin(address(0));
-            assertEq(actualPlugin, expectedPlugin, "plugin method installed");
+            assertEq(actualPlugin, expectedPlugin, "plugin method still installed");
         }
     }