Skip to content

fix(tests): align test contracts with new validation script rules #16561

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 2 commits into from
Jun 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -394,16 +394,11 @@ var excludedPaths = []string{
// - Enhancing the validation system to support libraries and complex structures
// - Fixing misspelled function names in test contracts
// - Restructuring tests to match actual function signatures
"test/libraries", // Libraries have different artifact structure, unsupported
"test/dispute/lib/LibPosition.t.sol", // Library testing - artifact structure issues
"test/L1/OptimismPortal2.t.sol", // Function name validation issues
"test/L1/ProxyAdminOwnedBase.t.sol", // Tests internal functions not in ABI
"test/L1/SystemConfig.t.sol", // Function name validation issues
"test/L2/OptimismMintableERC721.t.sol", // Function name validation issues
"test/L2/OptimismMintableERC721Factory.t.sol", // Function name validation issues
"test/L2/SequencerFeeVault.t.sol", // Function name validation issues
"test/L2/SuperchainERC20.t.sol", // Function name validation issues
"test/safe/SafeSigners.t.sol", // Function name validation issues
"test/libraries", // Libraries have different artifact structure, unsupported
"test/dispute/lib/LibPosition.t.sol", // Library testing - artifact structure issues
"test/L1/ProxyAdminOwnedBase.t.sol", // Tests internal functions not in ABI
"test/L1/SystemConfig.t.sol", // Tests internal functions not in ABI
"test/safe/SafeSigners.t.sol", // Function name validation issues

// PATHS EXCLUDED FROM TEST PATTERN VALIDATION:
// These paths are excluded because they don't follow the standard test contract
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,9 @@ contract OptimismPortal2_MigrateLiquidity_Test is CommonTest {
}
}

/// @title OptimismPortal2_MigrateToSuperRoot_Test
/// @title OptimismPortal2_MigrateToSuperRoots_Test
/// @notice Test contract for OptimismPortal2 `migrateToSuperRoots` function.
contract OptimismPortal2_MigrateToSuperRoot_Test is OptimismPortal2_TestInit {
contract OptimismPortal2_MigrateToSuperRoots_Test is OptimismPortal2_TestInit {
/// @notice Tests that `migrateToSuperRoots` reverts if the caller is not the proxy admin
/// owner.
function testFuzz_migrateToSuperRoots_notProxyAdminOwner_reverts(address _caller) external {
Expand Down
11 changes: 6 additions & 5 deletions packages/contracts-bedrock/test/L1/SystemConfig.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,9 @@ contract SystemConfig_Upgrade_Test is SystemConfig_TestInit {
}
}

/// @title SystemConfig_StartBlock_TestFail
/// @title SystemConfig_StartBlock_Test
/// @notice Test contract for SystemConfig `startBlock` function.
contract SystemConfig_StartBlock_TestFail is SystemConfig_TestInit {
contract SystemConfig_StartBlock_Test is SystemConfig_TestInit {
/// @notice Tests that startBlock is updated correctly when it's zero.
function test_startBlock_update_succeeds() external {
// Wipe out the initialized slot so the proxy can be initialized again
Expand Down Expand Up @@ -734,9 +734,10 @@ contract SystemConfig_Guardian_Test is SystemConfig_TestInit {
}
}

/// @notice This test is not testing any function directly from SystemConfig, but is indirectly
/// testing the `SuperchainConfig` inherited contract.
contract SystemConfig_Test is SystemConfig_TestInit {
/// @title SystemConfig_Uncategorized_Test
/// @notice General tests that are not testing any function directly of the `SystemConfig` contract
/// are testing multiple functions at once.
contract SystemConfig_SuperchainConfig_Test is SystemConfig_TestInit {
/// @notice Tests that `superchainConfig()` returns the correct address.
function test_superchainConfig_succeeds() external view {
assertEq(address(systemConfig.superchainConfig()), address(superchainConfig));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ contract OptimismMintableERC721_Burn_Test is OptimismMintableERC721_TestInit {
}
}

/// @title OptimismMintableERC721_SupportsInterfaces_Test
/// @notice Tests the `supportsInterfaces` function of the `OptimismMintableERC721` contract.
contract OptimismMintableERC721_SupportsInterfaces_Test is OptimismMintableERC721_TestInit {
/// @title OptimismMintableERC721_SupportsInterface_Test
/// @notice Tests the `supportsInterface` function of the `OptimismMintableERC721` contract.
contract OptimismMintableERC721_SupportsInterface_Test is OptimismMintableERC721_TestInit {
/// @notice Tests that the `supportsInterface` function returns true for
/// IOptimismMintableERC721, IERC721Enumerable, IERC721 and IERC165 interfaces.
function test_supportsInterfaces_succeeds() external view {
function test_supportsInterface_succeeds() external view {
// Checks if the contract supports the IOptimismMintableERC721 interface.
assertTrue(L2NFT.supportsInterface(type(IOptimismMintableERC721).interfaceId));
// Checks if the contract supports the IERC721Enumerable interface.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ contract OptimismMintableERC721Factory_Constructor_Test is OptimismMintableERC72
}
}

/// @title OptimismMintableERC721Factory_CreateMintableERC721_Test
/// @title OptimismMintableERC721Factory_CreateOptimismMintableERC721_Test
/// @notice Tests the `createOptimismMintableERC721` function of the
/// `OptimismMintableERC721Factory` contract.
contract OptimismMintableERC721Factory_CreateMintableERC721_Test is OptimismMintableERC721Factory_TestInit {
contract OptimismMintableERC721Factory_CreateOptimismMintableERC721_Test is OptimismMintableERC721Factory_TestInit {
/// @notice Tests that the `createOptimismMintableERC721` function succeeds.
function test_createOptimismMintableERC721_succeeds() external {
address remote = address(1234);
Expand Down
60 changes: 29 additions & 31 deletions packages/contracts-bedrock/test/L2/SequencerFeeVault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,31 @@ contract SequencerFeeVault_Receive_Test is SequencerFeeVault_TestInit {
/// @title SequencerFeeVault_Withdraw_Test
/// @notice Tests the `withdraw` function of the `SequencerFeeVault` contract.
contract SequencerFeeVault_Withdraw_Test is SequencerFeeVault_TestInit {
/// @notice Helper function to set up L2 withdrawal configuration.
function _setupL2Withdrawal() internal {
// Alter the deployment to use WithdrawalNetwork.L2
vm.etch(
EIP1967Helper.getImplementation(Predeploys.SEQUENCER_FEE_WALLET),
address(
DeployUtils.create1({
_name: "SequencerFeeVault",
_args: DeployUtils.encodeConstructor(
abi.encodeCall(
ISequencerFeeVault.__constructor__,
(
deploy.cfg().sequencerFeeVaultRecipient(),
deploy.cfg().sequencerFeeVaultMinimumWithdrawalAmount(),
Types.WithdrawalNetwork.L2
)
)
)
})
).code
);

recipient = deploy.cfg().sequencerFeeVaultRecipient();
}

/// @notice Tests that `withdraw` reverts if the balance is less than the minimum withdrawal
/// amount.
function test_withdraw_notEnough_reverts() external {
Expand Down Expand Up @@ -113,40 +138,11 @@ contract SequencerFeeVault_Withdraw_Test is SequencerFeeVault_TestInit {
assertEq(address(sequencerFeeVault).balance, 0);
assertEq(Predeploys.L2_TO_L1_MESSAGE_PASSER.balance, amount);
}
}

/// @title SequencerFeeVault_L2Withdrawal_Test
/// @notice Tests the `withdraw` function of the `SequencerFeeVault` contract for L2 withdrawals.
contract SequencerFeeVault_L2Withdrawal_Test is SequencerFeeVault_TestInit {
/// @notice Sets up the test suite with L2 withdrawal settings.
function setUp() public override {
super.setUp();

// Alter the deployment to use WithdrawalNetwork.L2
vm.etch(
EIP1967Helper.getImplementation(Predeploys.SEQUENCER_FEE_WALLET),
address(
DeployUtils.create1({
_name: "SequencerFeeVault",
_args: DeployUtils.encodeConstructor(
abi.encodeCall(
ISequencerFeeVault.__constructor__,
(
deploy.cfg().sequencerFeeVaultRecipient(),
deploy.cfg().sequencerFeeVaultMinimumWithdrawalAmount(),
Types.WithdrawalNetwork.L2
)
)
)
})
).code
);

recipient = deploy.cfg().sequencerFeeVaultRecipient();
}

/// @notice Tests that `withdraw` successfully initiates a withdrawal to L2.
function test_withdraw_toL2_succeeds() external {
_setupL2Withdrawal();

uint256 amount = sequencerFeeVault.MIN_WITHDRAWAL_AMOUNT() + 1;
vm.deal(address(sequencerFeeVault), amount);

Expand Down Expand Up @@ -174,6 +170,8 @@ contract SequencerFeeVault_L2Withdrawal_Test is SequencerFeeVault_TestInit {
/// @notice Tests that `withdraw` fails if the Recipient reverts. This also serves to simulate
/// a situation where insufficient gas is provided to the RECIPIENT.
function test_withdraw_toL2recipientReverts_fails() external {
_setupL2Withdrawal();

uint256 amount = sequencerFeeVault.MIN_WITHDRAWAL_AMOUNT();

vm.deal(address(sequencerFeeVault), amount);
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/test/L2/SuperchainERC20.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ contract SuperchainERC20_CrosschainBurn_Test is SuperchainERC20_TestInit {
}
}

/// @title SuperchainERC20_SupportsInterfaces_Test
/// @title SuperchainERC20_SupportsInterface_Test
/// @notice Tests the `supportsInterface` function of the `SuperchainERC20` contract.
contract SuperchainERC20_SupportsInterfaces_Test is SuperchainERC20_TestInit {
contract SuperchainERC20_SupportsInterface_Test is SuperchainERC20_TestInit {
/// @notice Tests that the `supportsInterface` function returns true for the `IERC7802`
/// interface.
function test_supportInterface_succeeds() public view {
Expand Down