test(runtime): Extend Deterministic Batcher Coverage#2518
test(runtime): Extend Deterministic Batcher Coverage#2518
Conversation
Co-authored-by: Codex <[email protected]>
Co-authored-by: Codex <[email protected]>
Co-authored-by: Codex <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// Spawn the polling loop on an injected runtime. | ||
| pub fn spawn_with_runtime<R: Runtime>(self, runtime: R) { | ||
| let task_runtime = runtime.clone(); | ||
| std::mem::drop(runtime.spawn(self.run(task_runtime))); | ||
| } |
There was a problem hiding this comment.
nit: spawn_with_runtime is declared pub but has no callers. If it's intended for future use in deterministic tests, consider adding a #[cfg(test)] gate or an #[allow(dead_code)] comment explaining the intent. Otherwise clippy (or cargo-udeps equivalent) may flag it.
| #[must_use] | ||
| pub fn critical_error(&self) -> Option<TxManagerError> { | ||
| let now = Instant::now(); | ||
| self.critical_error_with(None) | ||
| } | ||
|
|
||
| /// Returns the critical error using a runtime-relative timestamp. | ||
| #[must_use] | ||
| pub fn critical_error_at(&self, now: Duration) -> Option<TxManagerError> { | ||
| self.critical_error_with(Some(now)) |
There was a problem hiding this comment.
The no-arg critical_error() will silently miss an expired MempoolDeadline::Runtime deadline because runtime_now is None. Currently safe since all production call sites use critical_error_at(), but this is a latent footgun — a future caller using the simpler method would get silent deadline misses.
Consider either:
- Documenting on
critical_error()that it only checksWallClockdeadlines, or - Deprecating
critical_error()and requiring all callers to pass a timestamp, or - Having
critical_error()panic/warn if aRuntimedeadline is set (fail-fast rather than silent miss).
Review SummaryThe PR correctly abstracts Findings
No blocking issues found. The runtime abstraction is well-structured, the delegation pattern ( |
🟡 Heimdall Review Status
|
2 participants
Summary
This stacks on the initial deterministic runtime coverage PR and completes the tx-manager and batcher runtime testing pass. It moves additional tx-manager, L1 head source, and safe-head poller timer paths behind base-runtime, then adds virtual-time tests for event ordering, polling, cancellation, duplicate suppression, and timeout behavior.