chore: refactor proposal cancel tests [CLD-2769]#808
Conversation
… reused by other chain Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
# Conflicts: # e2e/tests/aptos/timelock_cancel.go # e2e/tests/ton/executable.go
Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
Signed-off-by: Pablo <pablo.estrada@smartcontract.com>
ecPablo
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors the timelock schedule→cancel E2E lifecycle into a reusable runner (e2e/tests/common) and updates multiple chain suites (Aptos, TON) to use it, while adding new E2E coverage for EVM and Solana.
Changes:
- Added a shared E2E schedule/cancel test runner with configurable hooks (
e2e/tests/common/timelock_schedule_cancel.go). - Updated Aptos and TON E2E tests to use the shared runner; added new E2E schedule/cancel tests for EVM and Solana.
- Added a similar schedule/cancel runner under
mcmspackage tests (but it currently diverges from the E2E runner in important correctness details).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test_runner_timelock_schedule_cancel_test.go | Adds a package-level test runner for schedule/cancel flow; currently has correctness issues for multi-op conversions and “not found” post-cancel behavior. |
| e2e/tests/common/timelock_schedule_cancel.go | Introduces the shared E2E runner and helper utilities for schedule/cancel lifecycle. |
| e2e/tests/aptos/timelock_cancel.go | Refactors Aptos cancel E2E test to use the shared runner and chain-specific metadata override. |
| e2e/tests/ton/executable.go | Switches TON schedule/cancel E2E coverage to use the shared runner. |
| e2e/tests/evm/timelock_schedule_cancel.go | Adds EVM E2E schedule/cancel test using the shared runner and EVM-specific finality/assertions. |
| e2e/tests/solana/timelock_schedule_cancel.go | Adds Solana E2E schedule/cancel test using the shared runner and Solana-specific wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There was a problem hiding this comment.
aren't we duplicating a lot of code now? It seems that test_runner_timelock_schedule_cancel_test.go and e2e/tests/common/timelock_schedule_cancel.go are very similar now.
There was a problem hiding this comment.
maybe timelock_executable_test.go::TestScheduleAndExecuteProposal is now redundant? At least partially?
| WaitForTransaction: func(ctx context.Context, t *testing.T, tx types.TransactionResult) { | ||
| t.Helper() | ||
| _, err := solana.SignatureFromBase58(tx.Hash) | ||
| require.NoError(t, err) | ||
| }, |
This pull request introduces new end-to-end tests for the "schedule and cancel" timelock proposal flow on EVM and Solana chains, and refactors existing tests for Aptos and TON chains to use a shared test harness in
e2e/tests/common. This change improves test coverage and consistency across chains while reducing code duplication.New E2E Tests:
TestScheduleAndCancelProposal, for EVM ine2e/tests/evm/timelock_schedule_cancel.go, covering scheduling and then canceling a timelock proposal and verifying no role is granted after cancellation.TestScheduleAndCancelProposal, for Solana ine2e/tests/solana/timelock_schedule_cancel.go, covering the analogous flow for Solana and ensuring proper cancellation.Refactoring for Code Reuse and Consistency:
e2ecommon.RunScheduleAndCancelTestharness and types frome2e/tests/common, replacing previous direct usage ofmcmslibtypes and helpers. This improves maintainability and standardizes test logic across chains. [1] [2] [3] [4] [5] [6] [7] [8]