Skip to content

refactor(test): cleanup of L2 test structure and documentation #15901

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

Merged
merged 34 commits into from
May 30, 2025

Conversation

aliersh
Copy link
Contributor

@aliersh aliersh commented May 13, 2025

This PR refactors a portion of the test suite within the /L2 folder as part of a broader effort to standardize structure, documentation, and formatting across the codebase.

Changes applied to each file:

  • Consolidated shared test initialization into dedicated *_TestInit contracts
  • Organized test functions into logically separated contracts inheriting from TestInit
  • Added @title and @notice tags to all test contracts
  • Added function-level @notice comments describing expected behavior under test
  • Replaced @dev tags with @notice where appropriate
  • Enforced a 100-character limit for inline comments

Progress

Refactored 25 of 25 test files (100.0% complete)

Note: GasPriceOracle.t.sol will be refactored in a later PR.

  • BaseFeeVault.t.sol
  • CrossL2Inbox.t.sol
  • CrossDomainOwnable.t.sol
  • CrossDomainOwnable2.t.sol
  • CrossDomainOwnable3.t.sol
  • ETHLiquidity.t.sol
  • GasPriceOracle.t.sol ⏸ Deferred
  • L1Block.t.sol
  • L1FeeVault.t.sol
  • L2CrossDomainMessenger.t.sol
  • L2ERC721Bridge.t.sol
  • L2StandardBridgeInterop.t.sol
  • L2StandardBridge.t.sol
  • L2ToL1MessagePasser.t.sol
  • L2ToL2CrossDomainMessenger.t.sol
  • OptimismMintableERC721.t.sol
  • OptimismMintableERC721Factory.t.sol
  • OptimismSuperchainERC20.t.sol
  • OptimismSuperchainERC20Beacon.t.sol
  • OptimismSuperchainERC20Factory.t.sol
  • OperatorFeeVault.t.sol
  • SequencerFeeVault.t.sol
  • SuperchainERC20.t.sol
  • SuperchainETHBridge.t.sol
  • SuperchainTokenBridge.t.sol
  • WETH.t.sol

aliersh added 16 commits May 9, 2025 15:37
- rename test contract to OperatorFeeVault_Constructor_Test
- add @title and @notice tags to test contract
- convert @dev tag to @notice
- rename test contract to L1FeeVault_Constructor_Test
- add @title and @notice tags to test contract
- convert @dev tag to @notice
- rename test contract to BaseFeeVault_Constructor_Test
- add @title and @notice tags to test contract
- convert @dev tag to @notice
…and documentation

- rename OptimismSuperchainERC20BeaconTest to OptimismSuperchainERC20Beacon_TestInit
- move implementation test into dedicated OptimismSuperchainERC20Beacon_Implementation_Test contract
- improve test contract documentation with more specific @title and @notice tags
- organize individual test functions into separate contracts
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
…and documentation

- consolidate test initialization into a single OptimismMintableERC721Factory_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- add function-level @notice comments describing expected behavior under test
- ensure comment lines stay within 100 character limit
… and documentation

- consolidate test initialization into a single OptimismSuperchainERC20Factory_TestInit contract
- organize individual test functions into separate contract inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit in OptimismSuperchainERC20Beacon test
- Update WETH test functions descriptions to explicitly mention default values
…ntation

- organize individual test functions into separate contracts
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
…cumentation

- consolidate test initialization into a single ExecutingMessageEmitted_TestInit contract
- rename ExecutingMessageEmittedTest to ExecutingMessageEmitted_Test
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single Predeploys_TestInit contract
- rename PredeploysTest and PredeploysInteropTest to Predeploys_Test and Predeploys_Interop_Test
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
…umentation

- consolidate test initialization into a single OptimismMintableERC721_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- group tests not directly tied to a base contract function into OptimismMintableERC721_Unclassified_Test
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- add function-level @notice comments describing expected behavior under test
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single SuperchainERC20_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
…ntation

- consolidate test initialization into a single SuperchainETHBridge_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single ETHLiquidity_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
@aliersh aliersh requested a review from a team as a code owner May 13, 2025 21:57
@aliersh aliersh requested a review from Inphi May 13, 2025 21:57
@aliersh aliersh marked this pull request as draft May 13, 2025 21:58
@aliersh
Copy link
Contributor Author

aliersh commented May 13, 2025

🔁 Continues work from the closed PR #15862, after renaming the branch.

aliersh added 8 commits May 14, 2025 10:59
…mentation

- consolidate test initialization into a single SuperchainTokenBridge_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single L1Block_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- add function-level @notice comments describing expected behavior under test
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single CrossL2Inbox_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- add @notice tag to test function comments
- ensure comment lines stay within 100 character limit
…cumentation

- consolidate test initialization into a single L2StandardBridgeInterop_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
- consolidate test initialization into a single L2ERC721Bridge_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- group tests not directly tied to a base contract function into L2ERC721Bridge_Unclassified_Test
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
…cumentation

- consolidate test initialization into a single OptimismSuperchainERC20_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- group tests not directly tied to a base contract function into OptimismSuperchainERC20_Unclassified_Test
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
…umentation

- consolidate test initialization into a single L2CrossDomainMessenger_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- group tests not directly tied to a base contract function into L2CrossDomainMessenger_Unclassified_Test
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
… documentation

- consolidate test initialization into a single L2ToL2CrossDomainMessenger_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- ensure comment lines stay within 100 character limit
aliersh added 7 commits May 15, 2025 18:01
- Update test initialization contract naming format to use _TestInit suffix
- Add backticks around contract names in documentation comments
- Fix duplicated notice tag in OptimismMintableERC721.t.sol
- Fix contract naming convention in ExecutingMessageEmitted.t.sol
- remove Predeploys.t.sol refactor
- remove ExecutingMessageEmmited.t.sol refactor
…tation

- consolidate test initialization into a single CrossDomainOwnable_TestInit contract
- organize tests into logical groups based on functionality (AccessControl and PortalIntegration)
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
…ntation

- consolidate test initialization into a single CrossDomainOwnable2_TestInit contract
- organize tests into logical groups (RevertConditions and SuccessConditions)
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
…ntation

- consolidate test initialization into a single CrossDomainOwnable3_TestInit contract
- organize tests into logical groups (BasicFunctionality, TransferOwnership, and CrossDomainAccess)
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
…tion

- consolidate test initialization into a single L2StandardBridge_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- group tests not directly tied to a base contract function into L2StandardBridge_Unclassified_Test
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
…ation

- consolidate test initialization into a single SequencerFeeVault_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- add @title and @notice tags to all test contracts
- convert @dev tags to @notice where appropriate
- ensure comment lines stay within 100 character limit
@aliersh aliersh marked this pull request as ready for review May 22, 2025 16:09
@smartcontracts
Copy link
Contributor

/ci authorize 61973cb

aliersh added 3 commits May 28, 2025 13:39
- rena test Initializer contract to AssetReceiver_TestInit contract
- organize individual test functions into separate contracts inheriting from TestInit
- refactor test function names to follow naming convention
- add @title and @notice tags to test contracts
- ensure comment stay within 100 character limit
@smartcontracts
Copy link
Contributor

/ci authorize 80f630e

@mds1 mds1 added this pull request to the merge queue May 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 29, 2025
@aliersh aliersh added this pull request to the merge queue May 30, 2025
Merged via the queue into ethereum-optimism:develop with commit ef93ca1 May 30, 2025
58 checks passed
@aliersh aliersh deleted the ari/l2-tests-cleanup branch May 30, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants