contracts-periphery: lint all test names

packages/contracts-bedrock/contracts/test/AdminFaucetAuthModule.t.sol

@@ -104,7 +104,7 @@ contract AdminFaucetAuthModuleTest is Test {
     /**
      * @notice assert that verify returns true for valid proofs signed by admins.
      */
-    function test_adminProof_verify_returnsTrue() external {
+    function test_adminProof_verify_succeeds() external {
         bytes32 nonce = faucetHelper.consumeNonce();
         address fundsReceiver = makeAddr("fundsReceiver");
         bytes memory proof = issueProofWithEIP712Domain(
@@ -132,7 +132,7 @@ contract AdminFaucetAuthModuleTest is Test {
     /**
      * @notice assert that verify returns false for proofs signed by nonadmins.
      */
-    function test_nonAdminProof_verify_returnsFalse() external {
+    function test_nonAdminProof_verify_succeeds() external {
         bytes32 nonce = faucetHelper.consumeNonce();
         address fundsReceiver = makeAddr("fundsReceiver");
         bytes memory proof = issueProofWithEIP712Domain(
@@ -161,7 +161,7 @@ contract AdminFaucetAuthModuleTest is Test {
      * @notice assert that verify returns false for proofs where the id in the proof is different
      * than the id in the call to verify.
      */
-    function test_proofWithWrongId_verify_returnsFalse() external {
+    function test_proofWithWrongId_verify_succeeds() external {
         bytes32 nonce = faucetHelper.consumeNonce();
         address fundsReceiver = makeAddr("fundsReceiver");
         address randomAddress = makeAddr("randomAddress");

packages/contracts-bedrock/contracts/test/AssetReceiver.t.sol

@@ -54,12 +54,12 @@ contract AssetReceiver_Initializer is Test {
 
 contract AssetReceiverTest is AssetReceiver_Initializer {
     // Tests if the owner was set correctly during deploy
-    function test_constructor() external {
+    function test_constructor_succeeds() external {
         assertEq(address(alice), assetReceiver.owner());
     }
 
     // Tests that receive works as inteded
-    function test_receive() external {
+    function test_receive_succeeds() external {
         // Check that contract balance is 0 initially
         assertEq(address(assetReceiver).balance, 0);
 
@@ -75,7 +75,7 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // Tests withdrawETH function with only an address as argument, called by owner
-    function test_withdrawETH() external {
+    function test_withdrawETH_succeeds() external {
         // Check contract initial balance
         assertEq(address(assetReceiver).balance, 0);
         // Fund contract with 1 eth and check caller and contract balances
@@ -97,14 +97,14 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // withdrawETH should fail if called by non-owner
-    function testFail_withdrawETH() external {
+    function test_withdrawETH_unauthorized_reverts() external {
         vm.deal(address(assetReceiver), 1 ether);
         assetReceiver.withdrawETH(payable(alice));
         vm.expectRevert("UNAUTHORIZED");
     }
 
     // Similar as withdrawETH but specify amount to withdraw
-    function test_withdrawETHwithAmount() external {
+    function test_withdrawETHwithAmount_succeeds() external {
         assertEq(address(assetReceiver).balance, 0);
 
         vm.deal(address(assetReceiver), 1 ether);
@@ -125,14 +125,14 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // withdrawETH with address and amount as arguments called by non-owner
-    function testFail_withdrawETHwithAmount() external {
+    function test_withdrawETHwithAmount_unauthorized_reverts() external {
         vm.deal(address(assetReceiver), 1 ether);
         assetReceiver.withdrawETH(payable(alice), 0.5 ether);
         vm.expectRevert("UNAUTHORIZED");
     }
 
     // Test withdrawERC20 with token and address arguments, from owner
-    function test_withdrawERC20() external {
+    function test_withdrawERC20_succeeds() external {
         // check balances before the call
         assertEq(testERC20.balanceOf(address(assetReceiver)), 0);
 
@@ -153,14 +153,14 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // Same as withdrawERC20 but call from non-owner
-    function testFail_withdrawERC20() external {
+    function test_withdrawERC20_unauthorized_reverts() external {
         deal(address(testERC20), address(assetReceiver), 100_000);
         assetReceiver.withdrawERC20(testERC20, alice);
         vm.expectRevert("UNAUTHORIZED");
     }
 
     // Similar as withdrawERC20 but specify amount to withdraw
-    function test_withdrawERC20withAmount() external {
+    function test_withdrawERC20withAmount_succeeds() external {
         // check balances before the call
         assertEq(testERC20.balanceOf(address(assetReceiver)), 0);
 
@@ -181,14 +181,14 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // Similar as withdrawERC20 with amount but call from non-owner
-    function testFail_withdrawERC20withAmount() external {
+    function test_withdrawERC20withAmount_unauthorized_reverts() external {
         deal(address(testERC20), address(assetReceiver), 100_000);
         assetReceiver.withdrawERC20(testERC20, alice, 50_000);
         vm.expectRevert("UNAUTHORIZED");
     }
 
     // Test withdrawERC721 from owner
-    function test_withdrawERC721() external {
+    function test_withdrawERC721_succeeds() external {
         // Check owner of the token before calling withdrawERC721
         assertEq(testERC721.ownerOf(DEFAULT_TOKEN_ID), alice);
 
@@ -209,7 +209,7 @@ contract AssetReceiverTest is AssetReceiver_Initializer {
     }
 
     // Similar as withdrawERC721 but call from non-owner
-    function testFail_withdrawERC721() external {
+    function test_withdrawERC721_unauthorized_reverts() external {
         vm.prank(alice);
         testERC721.transferFrom(alice, address(assetReceiver), DEFAULT_TOKEN_ID);
         assetReceiver.withdrawERC721(testERC721, alice, DEFAULT_TOKEN_ID);

packages/contracts-bedrock/contracts/test/AttestationStation.t.sol

@@ -28,7 +28,7 @@ contract AttestationStationTest is AttestationStation_Initializer {
         bytes val
     );
 
-    function test_attest_individual() external {
+    function test_attest_individual_succeeds() external {
         AttestationStation attestationStation = new AttestationStation();
 
         vm.expectEmit(true, true, true, true);
@@ -38,7 +38,7 @@ contract AttestationStationTest is AttestationStation_Initializer {
         attestationStation.attest({ _about: bob, _key: bytes32("foo"), _val: bytes("bar") });
     }
 
-    function test_attest_single() external {
+    function test_attest_single_succeeds() external {
         AttestationStation attestationStation = new AttestationStation();
 
         AttestationStation.AttestationData[]
@@ -100,7 +100,7 @@ contract AttestationStationTest is AttestationStation_Initializer {
         );
     }
 
-    function test_attest_bulk() external {
+    function test_attest_bulk_succeeds() external {
         AttestationStation attestationStation = new AttestationStation();
 
         vm.prank(alice_attestor);

packages/contracts-bedrock/contracts/test/CheckBalanceHigh.t.sol

@@ -43,7 +43,7 @@ contract CheckBalanceHighTest is Test {
      * @notice Fuzz the `check` function and assert that it always returns false
      *         when the target's balance is smaller than the threshold.
      */
-    function testFuzz_check_fails(address _target, uint256 _threshold) external {
+    function testFuzz_check_lowBalance_fails(address _target, uint256 _threshold) external {
         CheckBalanceHigh.Params memory p = CheckBalanceHigh.Params({
             target: _target,
             threshold: _threshold

packages/contracts-bedrock/contracts/test/CheckBalanceLow.t.sol

@@ -41,7 +41,7 @@ contract CheckBalanceLowTest is Test {
      * @notice Fuzz the `check` function and assert that it always returns false
      *         when the target's balance is larger than the threshold.
      */
-    function testFuzz_check_fails(address _target, uint256 _threshold) external {
+    function testFuzz_check_highBalance_fails(address _target, uint256 _threshold) external {
         CheckBalanceLow.Params memory p = CheckBalanceLow.Params({
             target: _target,
             threshold: _threshold

packages/contracts-bedrock/contracts/test/CheckGelatoLow.t.sol

@@ -78,7 +78,7 @@ contract CheckGelatoLowTest is Test {
      *         when the user's balance in the treasury is greater than or equal
      *         to the threshold.
      */
-    function testFuzz_check_fails(uint256 _threshold, address _recipient) external {
+    function testFuzz_check_highBalance_fails(uint256 _threshold, address _recipient) external {
         CheckGelatoLow.Params memory p = CheckGelatoLow.Params({
             treasury: address(gelato),
             threshold: _threshold,

packages/contracts-bedrock/contracts/test/Drippie.t.sol

@@ -149,7 +149,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Creates a drip and asserts that it was configured as expected.
      */
-    function test_create_success() external {
+    function test_create_succeeds() external {
         Drippie.DripConfig memory cfg = _defaultConfig();
         vm.expectEmit(true, true, true, true);
         emit DripCreated(dripName, dripName, cfg);
@@ -188,7 +188,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Ensures that the same drip cannot be created two times.
      */
-    function test_create_fails_twice() external {
+    function test_create_calledTwice_reverts() external {
         vm.startPrank(drippie.owner());
         Drippie.DripConfig memory cfg = _defaultConfig();
         drippie.create(dripName, cfg);
@@ -200,7 +200,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Ensures that only the owner of Drippie can create a drip.
      */
-    function testFuzz_fails_unauthorized(address caller) external {
+    function testFuzz_owner_unauthorized_reverts(address caller) external {
         vm.assume(caller != drippie.owner());
         vm.prank(caller);
         vm.expectRevert("UNAUTHORIZED");
@@ -210,7 +210,7 @@ contract Drippie_Test is Test {
     /**
      * @notice The owner should be able to set the status of the drip.
      */
-    function test_set_status_success() external {
+    function test_set_status_succeeds() external {
         vm.expectEmit(true, true, true, true);
         emit DripCreated(dripName, dripName, _defaultConfig());
         _createDefaultDrip(dripName);
@@ -258,7 +258,7 @@ contract Drippie_Test is Test {
     /**
      * @notice The drip status cannot be set back to NONE after it is created.
      */
-    function test_set_status_none_fails() external {
+    function test_set_statusNone_reverts() external {
         _createDefaultDrip(dripName);
 
         vm.prank(drippie.owner());
@@ -272,7 +272,7 @@ contract Drippie_Test is Test {
      * @notice The owner cannot set the status of the drip to the status that
      *         it is already set as.
      */
-    function test_set_status_same_fails() external {
+    function test_set_statusSame_reverts() external {
         _createDefaultDrip(dripName);
 
         vm.prank(drippie.owner());
@@ -286,7 +286,7 @@ contract Drippie_Test is Test {
      * @notice The owner should be able to archive the drip if it is in the
      *         paused state.
      */
-    function test_should_archive_if_paused_success() external {
+    function test_shouldArchive_ifPaused_succeeds() external {
         _createDefaultDrip(dripName);
 
         address owner = drippie.owner();
@@ -310,7 +310,7 @@ contract Drippie_Test is Test {
      * @notice The owner should not be able to archive the drip if it is in the
      *         active state.
      */
-    function test_should_not_archive_if_active_fails() external {
+    function test_shouldNotArchive_ifActive_reverts() external {
         _createDefaultDrip(dripName);
 
         vm.prank(drippie.owner());
@@ -327,7 +327,7 @@ contract Drippie_Test is Test {
      * @notice The owner should not be allowed to pause the drip if it
      *         has already been archived.
      */
-    function test_should_not_allow_paused_if_archived_fails() external {
+    function test_shouldNotAllowPaused_ifArchived_reverts() external {
         _createDefaultDrip(dripName);
 
         _notAllowFromArchive(dripName, Drippie.DripStatus.PAUSED);
@@ -337,7 +337,7 @@ contract Drippie_Test is Test {
      * @notice The owner should not be allowed to make the drip active again if
      *         it has already been archived.
      */
-    function test_should_not_allow_active_if_archived_fails() external {
+    function test_shouldNotAllowActive_ifArchived_reverts() external {
         _createDefaultDrip(dripName);
 
         _notAllowFromArchive(dripName, Drippie.DripStatus.ACTIVE);
@@ -361,7 +361,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Attempt to update a drip that does not exist.
      */
-    function test_name_not_exist_fails() external {
+    function test_name_notExist_reverts() external {
         string memory otherName = "bar";
 
         vm.prank(drippie.owner());
@@ -376,7 +376,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Expect a revert when attempting to set the status when not the owner.
      */
-    function test_status_unauthorized_fails() external {
+    function test_status_unauthorized_reverts() external {
         _createDefaultDrip(dripName);
 
         vm.expectRevert("UNAUTHORIZED");
@@ -386,7 +386,7 @@ contract Drippie_Test is Test {
     /**
      * @notice The drip should execute and be able to transfer value.
      */
-    function test_drip_amount() external {
+    function test_drip_amount_succeeds() external {
         _createDefaultDrip(dripName);
 
         vm.prank(drippie.owner());
@@ -418,7 +418,7 @@ contract Drippie_Test is Test {
     /**
      * @notice A single DripAction should be able to make a state modifying call.
      */
-    function test_trigger_one_function() external {
+    function test_trigger_oneFunction_succeeds() external {
         Drippie.DripConfig memory cfg = _defaultConfig();
 
         bytes32 key = bytes32(uint256(2));
@@ -455,7 +455,7 @@ contract Drippie_Test is Test {
     /**
      * @notice Multiple drip actions should be able to be triggered with the same check.
      */
-    function test_trigger_two_functions() external {
+    function test_trigger_twoFunctions_succeeds() external {
         Drippie.DripConfig memory cfg = _defaultConfig();
         Drippie.DripAction[] memory actions = new Drippie.DripAction[](2);
 
@@ -516,7 +516,7 @@ contract Drippie_Test is Test {
      *         trigger the same drip multiple times in the same interval. Then
      *         move forward to the next interval and it should trigger.
      */
-    function test_twice_in_one_interval_fails() external {
+    function test_twice_inOneInterval_reverts() external {
         _createDefaultDrip(dripName);
 
         vm.prank(drippie.owner());
@@ -551,7 +551,7 @@ contract Drippie_Test is Test {
      * @notice It should revert if attempting to trigger a drip that does not exist.
      *         Note that the drip was never created at the beginning of the test.
      */
-    function test_drip_not_exist_fails() external {
+    function test_drip_notExist_reverts() external {
         vm.prank(drippie.owner());
 
         vm.expectRevert("Drippie: selected drip does not exist or is not currently active");
@@ -562,7 +562,7 @@ contract Drippie_Test is Test {
     /**
      * @notice The owner cannot trigger the drip when it is paused.
      */
-    function test_not_active_fails() external {
+    function test_not_active_reverts() external {
         _createDefaultDrip(dripName);
 
         Drippie.DripStatus status = drippie.dripStatus(dripName);
@@ -598,7 +598,7 @@ contract Drippie_Test is Test {
      * @notice A non zero interval when reentrant is true will cause a revert
      *         when creating a drip.
      */
-    function test_reentrant_fails() external {
+    function test_drip_reentrant_reverts() external {
         address owner = drippie.owner();
         Drippie.DripConfig memory cfg = _defaultConfig();
         cfg.reentrant = true;
@@ -614,7 +614,7 @@ contract Drippie_Test is Test {
      * @notice If reentrant is false and the interval is 0 then it should
      *         revert when the drip is created.
      */
-    function test_non_reentrant_zero_interval_fails() external {
+    function test_notReentrant_zeroInterval_reverts() external {
         address owner = drippie.owner();
         Drippie.DripConfig memory cfg = _defaultConfig();
         cfg.reentrant = false;

packages/contracts-bedrock/contracts/test/Faucet.t.sol

@@ -126,7 +126,7 @@ contract Faucet_Initializer is Test {
 }
 
 contract FaucetTest is Faucet_Initializer {
-    function test_initialize() external {
+    function test_initialize_succeeds() external {
         assertEq(faucet.ADMIN(), faucetContractAdmin);
     }
 
@@ -181,7 +181,7 @@ contract FaucetTest is Faucet_Initializer {
         );
     }
 
-    function test_drip_optimistNft_sendsCorrectAmount() external {
+    function test_drip_optimistNftSendsCorrectAmount_succeeds() external {
         _enableFaucetAuthModules();
         bytes32 nonce = faucetHelper.consumeNonce();
         bytes memory signature = issueProofWithEIP712Domain(
@@ -213,7 +213,7 @@ contract FaucetTest is Faucet_Initializer {
         );
     }
 
-    function test_drip_github_sendsCorrectAmount() external {
+    function test_drip_githubSendsCorrectAmount_succeeds() external {
         _enableFaucetAuthModules();
         bytes32 nonce = faucetHelper.consumeNonce();
         bytes memory signature = issueProofWithEIP712Domain(
@@ -241,7 +241,7 @@ contract FaucetTest is Faucet_Initializer {
         );
     }
 
-    function test_drip_emitsEvent() external {
+    function test_drip_emitsEvent_succeeds() external {
         _enableFaucetAuthModules();
         bytes32 nonce = faucetHelper.consumeNonce();
         bytes memory signature = issueProofWithEIP712Domain(
@@ -300,7 +300,7 @@ contract FaucetTest is Faucet_Initializer {
         vm.stopPrank();
     }
 
-    function test_drip_preventsReplayAttacks() external {
+    function test_drip_preventsReplayAttacks_succeeds() external {
         _enableFaucetAuthModules();
         bytes32 nonce = faucetHelper.consumeNonce();
         bytes memory signature = issueProofWithEIP712Domain(
@@ -423,7 +423,7 @@ contract FaucetTest is Faucet_Initializer {
         vm.stopPrank();
     }
 
-    function test_withdraw_nonAdmin_fails() external {
+    function test_withdraw_nonAdmin_reverts() external {
         vm.prank(nonAdmin);
         vm.expectRevert("Faucet: function can only be called by admin");
         faucet.withdraw(payable(fundsReceiver), 2 ether);

packages/contracts-bedrock/contracts/test/Helpers.sol

@@ -10,10 +10,7 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
 import {
     ECDSAUpgradeable
 } from "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";
-import {
-    AdminFaucetAuthModule
-} from "../periphery/faucet/authmodules/AdminFaucetAuthModule.sol";
-
+import { AdminFaucetAuthModule } from "../periphery/faucet/authmodules/AdminFaucetAuthModule.sol";
 
 contract TestERC20 is ERC20 {
     constructor() ERC20("TEST", "TST", 18) {}

packages/contracts-bedrock/contracts/test/OptimistInviter.t.sol

@@ -260,7 +260,7 @@ contract OptimistInviter_Initializer is Test {
 }
 
 contract OptimistInviterTest is OptimistInviter_Initializer {
-    function test_initialize() external {
+    function test_initialize_succeeds() external {
         // expect attestationStation to be set
         assertEq(address(optimistInviter.ATTESTATION_STATION()), address(attestationStation));
         assertEq(optimistInviter.INVITE_GRANTER(), alice_inviteGranter);

packages/contracts-bedrock/contracts/test/Transactor.t.sol

@@ -35,12 +35,12 @@ contract Transactor_Initializer is Test {
 
 contract TransactorTest is Transactor_Initializer {
     // Tests if the owner was set correctly during deploy
-    function test_constructor() external {
+    function test_constructor_succeeds() external {
         assertEq(address(alice), transactor.owner());
     }
 
     // Tests CALL, should do a call to target
-    function test_CALL() external {
+    function test_call_succeeds() external {
         // Initialize call data
         bytes memory data = abi.encodeWithSelector(callRecorded.record.selector);
         // Run CALL
@@ -50,7 +50,7 @@ contract TransactorTest is Transactor_Initializer {
     }
 
     // It should revert if called by non-owner
-    function testFail_CALL() external {
+    function test_call_unauthorized_reverts() external {
         // Initialize call data
         bytes memory data = abi.encodeWithSelector(callRecorded.record.selector);
         // Run CALL
@@ -59,7 +59,7 @@ contract TransactorTest is Transactor_Initializer {
         vm.expectRevert("UNAUTHORIZED");
     }
 
-    function test_DELEGATECALL() external {
+    function test_delegateCall_succeeds() external {
         // Initialize call data
         bytes memory data = abi.encodeWithSelector(reverter.doRevert.selector);
         // Run CALL
@@ -69,7 +69,7 @@ contract TransactorTest is Transactor_Initializer {
     }
 
     // It should revert if called by non-owner
-    function testFail_DELEGATECALLL() external {
+    function test_delegateCall_unauthorized_reverts() external {
         // Initialize call data
         bytes memory data = abi.encodeWithSelector(reverter.doRevert.selector);
         // Run CALL

packages/contracts-bedrock/scripts/forge-test-names.ts

@@ -9,51 +9,51 @@ const checks: Array<{
   check: (parts: string[]) => boolean
   error: string
 }> = [
-  {
-    error: 'test name parts should be in camelCase',
-    check: (parts: string[]): boolean => {
-      return parts.every((part) => {
-        return part[0] === part[0].toLowerCase()
-      })
+    {
+      error: 'test name parts should be in camelCase',
+      check: (parts: string[]): boolean => {
+        return parts.every((part) => {
+          return part[0] === part[0].toLowerCase()
+        })
+      },
     },
-  },
-  {
-    error:
-      'test names should have either 3 or 4 parts, each separated by underscores',
-    check: (parts: string[]): boolean => {
-      return parts.length === 3 || parts.length === 4
+    {
+      error:
+        'test names should have either 3 or 4 parts, each separated by underscores',
+      check: (parts: string[]): boolean => {
+        return parts.length === 3 || parts.length === 4
+      },
     },
-  },
-  {
-    error: 'test names should begin with "test", "testFuzz", or "testDiff"',
-    check: (parts: string[]): boolean => {
-      return ['test', 'testFuzz', 'testDiff'].includes(parts[0])
+    {
+      error: 'test names should begin with "test", "testFuzz", or "testDiff"',
+      check: (parts: string[]): boolean => {
+        return ['test', 'testFuzz', 'testDiff'].includes(parts[0])
+      },
     },
-  },
-  {
-    error:
-      'test names should end with either "succeeds", "reverts", "fails", "works" or "benchmark[_num]"',
-    check: (parts: string[]): boolean => {
-      return (
-        ['succeeds', 'reverts', 'fails', 'benchmark', 'works'].includes(
-          parts[parts.length - 1]
-        ) ||
-        (parts[parts.length - 2] === 'benchmark' &&
-          !isNaN(parseInt(parts[parts.length - 1], 10)))
-      )
+    {
+      error:
+        'test names should end with either "succeeds", "reverts", "fails", "works" or "benchmark[_num]"',
+      check: (parts: string[]): boolean => {
+        return (
+          ['succeeds', 'reverts', 'fails', 'benchmark', 'works'].includes(
+            parts[parts.length - 1]
+          ) ||
+          (parts[parts.length - 2] === 'benchmark' &&
+            !isNaN(parseInt(parts[parts.length - 1], 10)))
+        )
+      },
     },
-  },
-  {
-    error:
-      'failure tests should have 4 parts, third part should indicate the reason for failure',
-    check: (parts: string[]): boolean => {
-      return (
-        parts.length === 4 ||
-        !['reverts', 'fails'].includes(parts[parts.length - 1])
-      )
+    {
+      error:
+        'failure tests should have 4 parts, third part should indicate the reason for failure',
+      check: (parts: string[]): boolean => {
+        return (
+          parts.length === 4 ||
+          !['reverts', 'fails'].includes(parts[parts.length - 1])
+        )
+      },
     },
-  },
-]
+  ]
 
 /**
  * Script for checking that all test functions are named correctly.
@@ -107,7 +107,7 @@ const main = async () => {
         // Check the rest.
         for (const { check, error } of checks) {
           if (!check(element.name.split('_'))) {
-            errors.push(`${filepath} function ${element.name}: ${error}`)
+            errors.push(`${filepath}#${element.name}: ${error}`)
             success = false
           }
         }