docs(ctb): Document the test naming convention

.changeset/wild-months-carry.md

@@ -0,0 +1,5 @@
+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Document test function naming convention and create a script for checking.

packages/contracts-bedrock/.gas-snapshot

@@ -8,7 +8,6 @@ GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (g
 GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 88513)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74953)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 36044)
-CrossDomainMessenger_Test:testFuzz_baseGas(uint32) (runs: 256, μ: 20196, ~: 20196)
 CrossDomainMessenger_Test:test_baseGas() (gas: 20054)
 CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner() (gas: 61806)
 CrossDomainOwnable_Test:test_onlyOwner() (gas: 34861)
@@ -19,7 +18,7 @@ CrossDomainOwnable2_Test:test_revertNotSetOnlyOwner2() (gas: 14565)
 CrossDomainOwnable2_Test:test_revertOnlyOwner() (gas: 61712)
 DeployerWhitelist_Test:test_owner() (gas: 7516)
 DeployerWhitelist_Test:test_storageSlots() (gas: 33395)
-Encoding_Test:test_nonceVersioning(uint240,uint16) (runs: 256, μ: 658, ~: 658)
+Encoding_Test:test_nonceVersioning_succeeds(uint240,uint16) (runs: 256, μ: 658, ~: 658)
 FeeVault_Test:test_constructor() (gas: 10623)
 FeeVault_Test:test_minWithdrawalAmount() (gas: 10689)
 GasPriceOracle_Test:test_baseFee() (gas: 8281)
@@ -29,7 +28,7 @@ GasPriceOracle_Test:test_overhead() (gas: 10568)
 GasPriceOracle_Test:test_scalar() (gas: 10610)
 GasPriceOracle_Test:test_setGasPriceReverts() (gas: 5888)
 GasPriceOracle_Test:test_setL1BaseFeeReverts() (gas: 5909)
-Hashing_Test:test_hashDepositSource() (gas: 628)
+Hashing_Test:test_hashDepositSource_succeeds() (gas: 650)
 L1BlockTest:test_basefee() (gas: 7531)
 L1BlockTest:test_hash() (gas: 7553)
 L1BlockTest:test_number() (gas: 7608)
@@ -122,9 +121,9 @@ L2StandardBridge_Test:test_receive() (gas: 131820)
 L2StandardBridge_Test:test_withdraw() (gas: 343951)
 L2StandardBridge_Test:test_withdrawTo() (gas: 344680)
 L2StandardBridge_Test:test_withdraw_onlyEOA() (gas: 251816)
-L2ToL1MessagePasserTest:test_burn() (gas: 112530)
-L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract() (gas: 70246)
-L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA() (gas: 75764)
+L2ToL1MessagePasserTest:test_burn_succeeds() (gas: 112508)
+L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract_succeeds() (gas: 70313)
+L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA_succeeds() (gas: 75764)
 LegacyERC20ETH_Test:test_approve() (gas: 10723)
 LegacyERC20ETH_Test:test_burn() (gas: 10615)
 LegacyERC20ETH_Test:test_crossDomain() (gas: 6339)
@@ -149,40 +148,40 @@ OptimismMintableTokenFactory_Test:test_bridge() (gas: 7577)
 OptimismMintableTokenFactory_Test:test_createStandardL2Token() (gas: 1095274)
 OptimismMintableTokenFactory_Test:test_createStandardL2TokenSameTwice() (gas: 2176691)
 OptimismMintableTokenFactory_Test:test_createStandardL2TokenShouldRevertIfRemoteIsZero() (gas: 9413)
-OptimismPortalUpgradeable_Test:test_initialize_cannotInitImpl_reverts() (gas: 10813)
-OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 15789)
-OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_success() (gas: 16033)
-OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_success() (gas: 180435)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 199750)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 202003)
+OptimismPortalUpgradeable_Test:test_initialize_cannotInitImpl_reverts() (gas: 10791)
+OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 15833)
+OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_succeeds() (gas: 16011)
+OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_succeeds() (gas: 180457)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 199728)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 202025)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 39656)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 197092)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 197814)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 177829)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 236177)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 237785)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_success() (gas: 229511)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 332126)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 193772)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 83498)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRecentWithdrawal_reverts() (gas: 177806)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 236200)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 237764)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 229467)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 332148)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 193816)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 83542)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onSelfCall_reverts() (gas: 50732)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_oninvalidWithdrawalProof_reverts() (gas: 136758)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_success() (gas: 277056)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 189117)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_success() (gas: 179324)
-OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17276)
-OptimismPortal_Test:test_OptimismPortalReceiveEth_success() (gas: 127480)
-OptimismPortal_Test:test_depositTransaction_NoValueContract_success() (gas: 76681)
-OptimismPortal_Test:test_depositTransaction_NoValueEOA_success() (gas: 77153)
-OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14245)
-OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_success() (gas: 76707)
-OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_success() (gas: 77029)
-OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_success() (gas: 83709)
-OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_success() (gas: 75872)
-OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_success() (gas: 83411)
-OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_success() (gas: 84111)
-OptimismPortal_Test:test_isOutputFinalized_success() (gas: 119473)
-OptimismPortal_Test:test_simple_isOutputFinalized_success() (gas: 24209)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_succeeds() (gas: 277079)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 189072)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 179324)
+OptimismPortal_Test:test_constructor_succeeds() (gas: 17277)
+OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14267)
+OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract_succeeds() (gas: 76662)
+OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA_succeeds() (gas: 77029)
+OptimismPortal_Test:test_depositTransaction_noValueContract_succeeds() (gas: 76681)
+OptimismPortal_Test:test_depositTransaction_noValueEOA_succeeds() (gas: 77198)
+OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation_succeeds() (gas: 83664)
+OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation_succeeds() (gas: 75862)
+OptimismPortal_Test:test_depositTransaction_withEthValueFromContract_succeeds() (gas: 83389)
+OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA_succeeds() (gas: 84133)
+OptimismPortal_Test:test_isOutputFinalized_succeeds() (gas: 119474)
+OptimismPortal_Test:test_receive_succeeds() (gas: 127522)
+OptimismPortal_Test:test_simple_isOutputFinalized_succeeds() (gas: 24165)
 Proxy_Test:test_clashingFunctionSignatures() (gas: 101347)
 Proxy_Test:test_implementationKey() (gas: 20887)
 Proxy_Test:test_implementationProxyCallIfNotAdmin() (gas: 29963)

packages/contracts-bedrock/README.md

@@ -194,3 +194,48 @@ After the initial Bedrock upgrade, contracts MUST use the following versioning s
 #### Exceptions
 
 We have made an exception to the `Semver` rule for the `WETH` contract to avoid making changes to a well-known, simple, and recognizable contract.
+
+### Tests
+
+Tests are written using Foundry.
+
+All test contracts and functions should be organized and named according to the following guidelines.
+
+These guidelines are also encoded in a script which can be run with:
+
+```
+ts-node scripts/forge-test-names.ts
+```
+
+*Note: This is a work in progress, not all test files are compliant with these guidelines.*
+
+#### Organizing Principles
+
+- Solidity `contract`s are used to organize the test suite similar to how mocha uses describe.
+- Every non-trivial state changing function should have a separate contract for happy and sad path
+   tests. This helps to make it very obvious where there are not yet sad path tests.
+- Simpler functions like getters and setters are grouped together into test contracts.
+
+#### Test function naming convention
+
+Test function names are split by underscores, into 3 or 4 parts. An example function name is `test_onlyOwner_callerIsNotOwner_reverts()`.
+
+The parts are: `[method]_[FunctionName]_[reason]_[success]`, where:
+
+- `[method]` is either `test`, `testFuzz`, or `testDiff`
+- `[FunctionName]` is the name of the function or higher level behavior being tested.
+- `[reason]` is an optional description for the behavior being tested.
+- `[status]` must be one of:
+  - `succeeds`: used for most happy path cases
+  - `reverts`: used for most sad path cases
+  - `works`: used for tests which include a mix of happy and sad assertions (these should be broken up if possible)
+  - `fails`: used for tests which 'fail' in some way other than reverting
+  - `benchmark`: used for tests intended to establish gas costs
+
+#### Contract Naming Conventions
+
+Test contracts should be named one of the following according to their use:
+
+- `TargetContract_Init` for contracts that perform basic setup to be reused in other test contracts.
+- `TargetContract_Function_Test` for contracts containing happy path tests for a given function.
+- `TargetContract_Function_TestFail` for contracts containing sad path tests for a given function.

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

@@ -5,7 +5,7 @@ import { Test } from "forge-std/Test.sol";
 import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
 
 contract AddressAliasHelper_Test is Test {
-    function test_fuzz_roundtrip(address _address) external {
+    function testFuzz_applyAndUndo_succeeds(address _address) external {
         address aliased = AddressAliasHelper.applyL1ToL2Alias(_address);
         address unaliased = AddressAliasHelper.undoL1ToL2Alias(aliased);
         assertEq(_address, unaliased);

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

@@ -10,15 +10,15 @@ contract Encoding_Test is CommonTest {
         _setUp();
     }
 
-    function test_nonceVersioning(uint240 _nonce, uint16 _version) external {
+    function test_nonceVersioning_succeeds(uint240 _nonce, uint16 _version) external {
         (uint240 nonce, uint16 version) = Encoding.decodeVersionedNonce(
             Encoding.encodeVersionedNonce(_nonce, _version)
         );
         assertEq(version, _version);
         assertEq(nonce, _nonce);
     }
 
-    function test_decodeVersionedNonce_differential(uint240 _nonce, uint16 _version) external {
+    function testDiff_decodeVersionedNonce_succeeds(uint240 _nonce, uint16 _version) external {
         uint256 nonce = uint256(Encoding.encodeVersionedNonce(_nonce, _version));
         (uint256 decodedNonce, uint256 decodedVersion) = ffi.decodeVersionedNonce(nonce);
 
@@ -27,7 +27,7 @@ contract Encoding_Test is CommonTest {
         assertEq(_nonce, uint240(decodedNonce));
     }
 
-    function test_encodeCrossDomainMessage_differential(
+    function testDiff_encodeCrossDomainMessage_succeeds(
         uint240 _nonce,
         uint8 _version,
         address _sender,
@@ -60,7 +60,7 @@ contract Encoding_Test is CommonTest {
         assertEq(encoding, _encoding);
     }
 
-    function test_encodeDepositTransaction_differential(
+    function testDiff_encodeDepositTransaction_succeeds(
         address _from,
         address _to,
         uint256 _mint,

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

@@ -11,7 +11,7 @@ contract Hashing_Test is CommonTest {
         _setUp();
     }
 
-    function test_hashDepositSource() external {
+    function test_hashDepositSource_succeeds() external {
         bytes32 sourceHash = Hashing.hashDepositSource(
             0xd25df7858efc1778118fb133ac561b138845361626dfb976699c5287ed0f4959,
             0x1
@@ -20,7 +20,7 @@ contract Hashing_Test is CommonTest {
         assertEq(sourceHash, 0xf923fb07134d7d287cb52c770cc619e17e82606c21a875c92f4c63b65280a5cc);
     }
 
-    function test_hashCrossDomainMessage_differential(
+    function testDiff_hashCrossDomainMessage_succeeds(
         uint240 _nonce,
         uint16 _version,
         address _sender,
@@ -54,7 +54,7 @@ contract Hashing_Test is CommonTest {
         assertEq(hash, _hash);
     }
 
-    function test_hashWithdrawal_differential(
+    function testDiff_hashWithdrawal_succeeds(
         uint256 _nonce,
         address _sender,
         address _target,
@@ -71,7 +71,7 @@ contract Hashing_Test is CommonTest {
         assertEq(hash, _hash);
     }
 
-    function test_hashOutputRootProof_differential(
+    function testDiff_hashOutputRootProof_succeeds(
         bytes32 _version,
         bytes32 _stateRoot,
         bytes32 _messagePasserStorageRoot,
@@ -98,7 +98,7 @@ contract Hashing_Test is CommonTest {
 
     // TODO(tynes): foundry bug cannot serialize
     // bytes32 as strings with vm.toString
-    function test_hashDepositTransaction_differential(
+    function testDiff_hashDepositTransaction_succeeds(
         address _from,
         address _to,
         uint256 _mint,

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

@@ -25,7 +25,7 @@ contract L2ToL1MessagePasserTest is CommonTest {
         messagePasser = new L2ToL1MessagePasser();
     }
 
-    function test_fuzz_initiateWithdrawal(
+    function testFuzz_initiateWithdrawal_succeeds(
         address _sender,
         address _target,
         uint256 _value,
@@ -60,7 +60,7 @@ contract L2ToL1MessagePasserTest is CommonTest {
     }
 
     // Test: initiateWithdrawal should emit the correct log when called by a contract
-    function test_initiateWithdrawal_fromContract() external {
+    function test_initiateWithdrawal_fromContract_succeeds() external {
         bytes32 withdrawalHash = Hashing.hashWithdrawal(
             Types.WithdrawalTransaction(
                 messagePasser.nonce(),
@@ -88,7 +88,7 @@ contract L2ToL1MessagePasserTest is CommonTest {
     }
 
     // Test: initiateWithdrawal should emit the correct log when called by an EOA
-    function test_initiateWithdrawal_fromEOA() external {
+    function test_initiateWithdrawal_fromEOA_succeeds() external {
         uint256 gasLimit = 64000;
         address target = address(4);
         uint256 value = 100;
@@ -114,7 +114,7 @@ contract L2ToL1MessagePasserTest is CommonTest {
     }
 
     // Test: burn should destroy the ETH held in the contract
-    function test_burn() external {
+    function test_burn_succeeds() external {
         messagePasser.initiateWithdrawal{ value: NON_ZERO_VALUE }(
             NON_ZERO_ADDRESS,
             NON_ZERO_GASLIMIT,