This repository was archived by the owner on Jan 16, 2026. It is now read-only.
Conversation
theochap
requested review from
clabby,
dhyaniarun1993,
emhane,
itschaindev,
refcell and
sadiq1971
as code owners
theochap
added
K-feature
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
theochap
force-pushed
the
theo/restart-test
branch
from
94f90e9 to
d1f202f
Compare
clabby
approved these changes
Jul 24, 2025
Contributor
clabby
left a comment
clabby
left a comment
There was a problem hiding this comment.
very nice. g2g if CI passes
theochap
force-pushed
the
theo/add-engine-queue-test
branch
from
4990f3f to
b0e904d
Compare
theochap
force-pushed
the
theo/restart-test
branch
from
d1f202f to
e085998
Compare
theochap
force-pushed
the
theo/add-engine-queue-test
branch
3 times, most recently
from
2923d27 to
e99a0fb
Compare
theochap
force-pushed
the
theo/restart-test
branch
from
e085998 to
adf8e72
Compare
theochap
force-pushed
the
theo/add-engine-queue-test
branch
2 times, most recently
from
e03cc19 to
8c886a3
Compare
theochap
force-pushed
the
theo/restart-test
branch
from
adf8e72 to
15d28cf
Compare
theochap
force-pushed
the
theo/restart-test
branch
from
15d28cf to
efbb929
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds restart tests to validate that nodes can recover from stopping and restarting after extended periods. These tests ensure node resilience and proper p2p reconnection capabilities after downtime.
- Introduces
TestRestartSyncto verify nodes can resync after 2-minute downtime - Adds
TestRestartP2pto confirm p2p connections are re-established after restart - Implements node stop/start functionality using Kurtosis CLI commands
- Enables dev RPC endpoints across all test network configurations for enhanced testing capabilities
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/node/restart_test.go | New test file with sync and p2p restart validation tests |
| tests/node/mod.go | Utility functions for node control, WebSocket handling, and dev RPC support |
| tests/node/engine_test.go | New engine queue monitoring test using dev RPC endpoints |
| tests/node/rpc_test.go | Simplified peer selection logic removing connectedness check |
| tests/justfile | Extended test timeout and added filtered test execution capability |
| tests/devnets/*.yaml | Enabled dev RPC endpoints for all Kona node configurations |
Comments suppressed due to low confidence (1)
tests/node/mod.go:49
- [nitpick] The generic type parameter
Outis not descriptive. Consider using a more specific name likeResultTypeorTResultto improve readability.
Result Out `json:"result"`
tests/node/restart_test.go
Outdated
| var wg sync.WaitGroup | ||
| for _, node := range nodes { | ||
| if node == ref { | ||
| t.Log("skipping reference node %s", node.Escape().ID().Key()) |
There was a problem hiding this comment.
The Log statement uses incorrect format specifier. It should use t.Logf with the format string or remove the format placeholder.
Suggested change
| t.Log("skipping reference node %s", node.Escape().ID().Key()) | |
| t.Logf("skipping reference node %s", node.Escape().ID().Key()) |
tests/node/restart_test.go
Outdated
| found := false | ||
| for _, peer := range peers.Peers { | ||
| if peer.NodeID == refId { | ||
| t.Log("node %s is connected to reference node %s", clName, ref.Escape().ID().Key()) |
There was a problem hiding this comment.
The Log statement uses incorrect format specifier. It should use t.Logf with the format string or remove the format placeholder.
Suggested change
| t.Log("node %s is connected to reference node %s", clName, ref.Escape().ID().Key()) | |
| t.Logf("node %s is connected to reference node %s", clName, ref.Escape().ID().Key()) |
theochap
force-pushed
the
theo/restart-test
branch
from
80d3bc4 to
76e2fa6
Compare
theochap
force-pushed
the
theo/restart-test
branch
from
76e2fa6 to
3542838
Compare
Member
Author
|
It seems that manually restarting the kurtosis services cause segfault errors in the devstack... Going to close this one until we integrate kona in sysgo |
auto-merge was automatically disabled
July 25, 2025 21:05
Pull request was closed
theochap
added a commit
to ethereum-optimism/optimism
that referenced
this pull request
Dec 10, 2025
## Description This PR adds back the connection drop test and the restart test inside our e2e testing suite See op-rs/kona#2554 and op-rs/kona#2553
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a couple of restart tests to ensure that nodes can recover from stopping and restarting after some time