-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add backward, forward let test #231
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a comprehensive end-to-end test for the backwardLET and forwardLET features, which are emergency bridge contract functions that allow rolling back and restoring deposit counts and Local Exit Root (LER) states. The PR also removes the obsolete e2e-pp.bats test file and consolidates functionality by adding a service management helper function.
Key changes:
- Adds a new comprehensive test that validates backwardLET (rolling back deposits) and forwardLET (restoring deposits) functionality through a multi-step bridge flow
- Removes the redundant e2e-pp.bats certificate settlement test
- Adds a reusable manage_kurtosis_service helper function for starting/stopping Kurtosis services
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/aggkit/e2e-pp.bats | Deleted obsolete certificate settlement test that is redundant with other test coverage |
| tests/aggkit/bridge-sovereign-chain-e2e.bats | Adds comprehensive backwardLET/forwardLET test with multiple bridge operations, emergency state management, and certificate settlement verification |
| core/helpers/agglayer-cdk-common-setup.bash | Adds manage_kurtosis_service helper function and integrates bridge service management into common setup |
| TESTSINVENTORY.md | Updates test inventory with new backwardLET/forwardLET test entry and removes obsolete certificate settlement test reference |
| .github/workflows/aggkit-e2e-single-chain.yml | Removes reference to deleted e2e-pp.bats test from workflow and fixes trailing whitespace |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| local deposit_count_after_forward="$output" | ||
| log "Deposit count after forwardLET: $deposit_count_after_forward" | ||
| assert_equal "$deposit_count_after_forward" "$((last_deposit_count + 1))" | ||
| log "Deposit count restored correctly after forwardLET" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also worth checking the aggkit db i think after forwardLET
| manage_kurtosis_service "stop" "zkevm-bridge-service-001" | ||
| manage_kurtosis_service "start" "bridge-spammer-001" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to move this part to tear down of this specific test, so that it gets executed even though the test fail at some earlier point? My reasoning is to give chance for other tests to execute succesfully in case fail-fast env variable was set to false (and we try to execute as much tests as possible).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc, teardown() works for all the func, how can we execute separately for each test?
agglayer/aggkit#1447