Skip to content

Commit cff52c2

Browse files
kariyclaude
andcommitted
test(messaging): server lifecycle coverage (tier A.3)
Adds the three lifecycle tests that were skipped earlier because the old hard-coded `pool: TxPool` field made `MessagingServer` impossible to construct from `katana-messaging`'s own dep graph (a `TxValidator` needs a state provider, block env, etc.). With the generic-pool refactor in the prior commit, a `NoopValidator`-backed pool is enough to satisfy the trait bounds and exercise: - start() rejects a second call on the same instance - a clone cannot be started (rewind_rx was scrubbed on clone) - the drain task survives a fully-closed rewind channel Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6f27753 commit cff52c2

1 file changed

Lines changed: 120 additions & 0 deletions

File tree

crates/messaging/src/server.rs

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,12 +375,39 @@ impl MessagingHandle {
375375

376376
#[cfg(test)]
377377
mod tests {
378+
use std::time::Duration;
379+
380+
use katana_pool::ordering::FiFo;
381+
use katana_pool::pool::Pool;
382+
use katana_pool::validation::NoopValidator;
383+
use katana_primitives::transaction::ExecutableTxWithHash;
378384
use katana_primitives::Felt;
379385
use katana_provider::api::messaging::MessagingL1ToL2IndexProvider;
380386
use katana_provider::DbProviderFactory;
387+
use url::Url;
381388

382389
use super::*;
383390

391+
/// No-op pool used by the lifecycle tests. The drain task never actually inserts
392+
/// transactions in these tests (the configured settlement endpoint is unroutable),
393+
/// so this type only needs to satisfy the trait bounds of `start()`.
394+
type NoopPool =
395+
Pool<ExecutableTxWithHash, NoopValidator<ExecutableTxWithHash>, FiFo<ExecutableTxWithHash>>;
396+
397+
fn noop_pool() -> NoopPool {
398+
Pool::new(NoopValidator::new(), FiFo::new())
399+
}
400+
401+
/// Settlement config pointing at a non-routable URL. The drain task may try
402+
/// `latest_block()` against this; it'll fail or pend, which is fine — the
403+
/// lifecycle tests don't depend on any successful gather.
404+
fn unroutable_settlement() -> SettlementChainConfig {
405+
SettlementChainConfig::Ethereum {
406+
rpc_url: Url::parse("http://127.0.0.1:1/").unwrap(),
407+
contract_address: Default::default(),
408+
}
409+
}
410+
384411
#[test]
385412
fn resume_cursor_falls_back_to_default_from_block_when_no_checkpoint_persisted() {
386413
let provider = DbProviderFactory::new_in_memory();
@@ -497,4 +524,97 @@ mod tests {
497524
assert_eq!(cp.block, 10, "checkpoint should reflect the latest committed message");
498525
assert_eq!(cp.tx_index, 2);
499526
}
527+
528+
// -------------------------------------------------------------------------
529+
// Lifecycle tests
530+
//
531+
// These exercise `MessagingServer::start` itself — that the rewind_rx is
532+
// single-take, that clones can't be started, and that a closed rewind
533+
// channel doesn't kill the drain task. They use a non-routable settlement
534+
// endpoint; the drain task never produces work but stays alive, which is
535+
// all the lifecycle invariants require.
536+
// -------------------------------------------------------------------------
537+
538+
/// The `rewind_rx` is taken on first `start()`; a second call on the same
539+
/// instance must fail with a clear "already started" error rather than
540+
/// silently spawning a second drain task that competes for rewind signals.
541+
#[tokio::test]
542+
async fn start_twice_returns_error() {
543+
let provider = DbProviderFactory::new_in_memory();
544+
let pool = noop_pool();
545+
let mut server = MessagingServer::new(ChainId::default(), pool, provider)
546+
.settlement(unroutable_settlement())
547+
.interval(60);
548+
549+
let mut handle = server.start().expect("first start succeeds");
550+
551+
let err = server.start().expect_err("second start must fail");
552+
let msg = err.to_string();
553+
assert!(msg.contains("already started"), "expected 'already started' in error, got: {msg}");
554+
555+
// Clean up the first task so the test process exits cleanly.
556+
handle.stop();
557+
handle.stopped().await;
558+
}
559+
560+
/// `Clone for MessagingServer` deliberately sets `rewind_rx: None` on the
561+
/// clone so only the original instance can drive the drain loop. Starting
562+
/// a clone must fail with the same error as a double-start.
563+
#[tokio::test]
564+
async fn clone_cannot_be_started() {
565+
let provider = DbProviderFactory::new_in_memory();
566+
let pool = noop_pool();
567+
let server = MessagingServer::new(ChainId::default(), pool, provider)
568+
.settlement(unroutable_settlement())
569+
.interval(60);
570+
571+
let mut clone = server.clone();
572+
573+
let err = clone.start().expect_err("starting a clone must fail");
574+
let msg = err.to_string();
575+
assert!(msg.contains("already started"), "expected 'already started' in error, got: {msg}");
576+
577+
// The original is still startable (rewind_rx wasn't taken from it).
578+
let mut original = server;
579+
let mut handle = original.start().expect("original is still startable after cloning");
580+
handle.stop();
581+
handle.stopped().await;
582+
}
583+
584+
/// Dropping the controller (and hence one rewind_tx sender) must not kill
585+
/// the running drain task. The other arms of the `select!` (shutdown,
586+
/// messenger.next) keep firing; the rewind arm just goes permanently
587+
/// inactive once all senders are gone. This guards against a regression
588+
/// where the loop would exit on `rewind_rx.recv() == None`.
589+
#[tokio::test]
590+
async fn rewind_sender_dropped_does_not_kill_task() {
591+
let provider = DbProviderFactory::new_in_memory();
592+
let pool = noop_pool();
593+
let mut server = MessagingServer::new(ChainId::default(), pool, provider)
594+
.settlement(unroutable_settlement())
595+
.interval(60);
596+
597+
let controller = server.controller();
598+
let mut handle = server.start().expect("start succeeds");
599+
600+
// Drop everything that holds a rewind_tx clone: the controller, the
601+
// server itself (its own sender), so the receiver inside the task
602+
// observes a fully-closed channel.
603+
drop(controller);
604+
drop(server);
605+
606+
// Give the runtime a moment to deliver the channel-closed notification
607+
// to the task. Tokio's `mpsc::Receiver::recv` returns `None` once all
608+
// senders are dropped, and the `select!` arm with that pattern simply
609+
// never matches again — the other arms must keep working.
610+
tokio::time::sleep(Duration::from_millis(50)).await;
611+
612+
assert!(
613+
!handle.task_handle.is_finished(),
614+
"drain task must survive a closed rewind channel"
615+
);
616+
617+
handle.stop();
618+
handle.stopped().await;
619+
}
500620
}

0 commit comments

Comments
 (0)