From 282fcc3d013a6b9c4dde778fa00f4a17a7e3c2d6 Mon Sep 17 00:00:00 2001 From: zacheryasc Date: Sat, 7 Feb 2026 17:29:01 +0000 Subject: [PATCH] refactor: better tests (#18) Still unsatisfied, but these are better than before. --- .gitignore | 3 +- src/delivery.rs | 48 -------- tests/runtime_api.rs | 161 ++++++++++++++++++++++++++ tests/{stats_demo.rs => stats_api.rs} | 59 ++-------- tests/test_python.py | 47 +++----- 5 files changed, 189 insertions(+), 129 deletions(-) rename tests/{stats_demo.rs => stats_api.rs} (51%) diff --git a/.gitignore b/.gitignore index eff640b..6d25471 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ tools/depgraph/target/ node_modules/ .vscode/ -.venv \ No newline at end of file +.venv +__pycache__ \ No newline at end of file diff --git a/src/delivery.rs b/src/delivery.rs index ec520b1..6ed7fc7 100644 --- a/src/delivery.rs +++ b/src/delivery.rs @@ -168,51 +168,3 @@ pub(crate) struct TickContext<'a> { pub(crate) inbox_registry: &'a InboxRegistry, pub(crate) config: &'a RuntimeConfig, } - -#[cfg(test)] -mod address_map_tests { - use super::*; - - #[test] - fn insert_and_lookup() { - let map = AddressMap::new(); - let addr = ActorAddress::default(); - let wid = WorkerId(3); - map.insert(addr, wid); - assert_eq!(map.lookup(&addr), Some(wid)); - } - - #[test] - fn lookup_missing_returns_none() { - let map = AddressMap::new(); - let addr = ActorAddress::default(); - assert_eq!(map.lookup(&addr), None); - } - - #[test] - fn remove_works() { - let map = AddressMap::new(); - let addr = ActorAddress::default(); - map.insert(addr, WorkerId(0)); - map.remove(&addr); - assert_eq!(map.lookup(&addr), None); - } - - #[test] - fn len_tracks_entries() { - let map = AddressMap::with_capacity(10); - assert_eq!(map.len(), 0); - let addr1 = ActorAddress::default(); - map.insert(addr1, WorkerId(0)); - assert_eq!(map.len(), 1); - } - - #[test] - fn round_robin() { - let p = Placement::new(3); - assert_eq!(p.next_worker(), WorkerId(0)); - assert_eq!(p.next_worker(), WorkerId(1)); - assert_eq!(p.next_worker(), WorkerId(2)); - assert_eq!(p.next_worker(), WorkerId(0)); - } -} diff --git a/tests/runtime_api.rs b/tests/runtime_api.rs index 88238a5..824f2f2 100644 --- a/tests/runtime_api.rs +++ b/tests/runtime_api.rs @@ -27,6 +27,52 @@ impl ActorInterface for EchoActor { } } +/// Child actor that doubles the payload and replies. +struct DoubleActor; + +#[derive(Clone)] +struct DoubleRequest { + value: usize, + reply_to: ActorAddress, +} + +#[derive(Clone, Debug, PartialEq)] +struct DoubleResponse(usize); + +impl ActorInterface for DoubleActor { + type Incoming = DoubleRequest; + type Response = DoubleResponse; + + fn handle(&mut self, ctx: &Ctx, msg: DoubleRequest) { + let _ = ctx.send(msg.reply_to, DoubleResponse(msg.value * 2)); + } +} + +/// Parent actor that spawns a DoubleActor child and delegates work. +struct DelegateActor; + +#[derive(Clone)] +struct DelegateRequest { + value: usize, + reply_to: ActorAddress, +} + +impl ActorInterface for DelegateActor { + type Incoming = DelegateRequest; + type Response = (); + + fn handle(&mut self, ctx: &Ctx, msg: DelegateRequest) { + let child = ctx.spawn(DoubleActor).expect("spawn child"); + let _ = ctx.send( + child, + DoubleRequest { + value: msg.value, + reply_to: msg.reply_to, + }, + ); + } +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -95,3 +141,118 @@ fn test_multi_thread_spawn_actor_and_inbox() { let result = check.join().unwrap(); assert_eq!(result, Some(EchoResponse(99))); } + +// --------------------------------------------------------------------------- +// Behavioral story tests +// --------------------------------------------------------------------------- + +#[test] +fn send_to_unknown_address_fails() { + let rt = Runtime::new(RuntimeConfig::default()); + let bogus = ActorAddress::new_random(); + let result = rt.send_to(bogus, 42u64); + assert!(result.is_err()); +} + +#[test] +fn actor_spawns_child_and_delegates() { + // Uses 2 workers so parent and child land on different workers, + // avoiding the single-worker timing issue where pending_local + // delivery precedes spawn-queue draining. + let config = RuntimeConfig { + num_threads: 2, + ..Default::default() + }; + let rt = Runtime::new(config); + + let parent = rt.spawn(DelegateActor).expect("spawn parent"); + let inbox: Inbox = rt.new_inbox().unwrap(); + + rt.send_to( + parent, + DelegateRequest { + value: 7, + reply_to: *inbox.addr(), + }, + ) + .unwrap(); + + let handle = rt.run().unwrap(); + + let check = std::thread::spawn(move || { + for _ in 0..100 { + std::thread::sleep(std::time::Duration::from_millis(10)); + if let Some(resp) = inbox.try_recv() { + handle.shutdown(); + return Some(resp); + } + } + handle.shutdown(); + None + }); + + let result = check.join().unwrap(); + assert_eq!(result, Some(DoubleResponse(14))); +} + +#[test] +fn multiple_actors_independent_mailboxes() { + let rt = Runtime::new(RuntimeConfig::default()); + + let inbox_a: Inbox = rt.new_inbox().unwrap(); + let inbox_b: Inbox = rt.new_inbox().unwrap(); + let inbox_c: Inbox = rt.new_inbox().unwrap(); + + let actor_a = rt.spawn(EchoActor).unwrap(); + let actor_b = rt.spawn(EchoActor).unwrap(); + let actor_c = rt.spawn(EchoActor).unwrap(); + + rt.send_to(actor_a, EchoMessage { payload: 10, reply_to: *inbox_a.addr() }).unwrap(); + rt.send_to(actor_b, EchoMessage { payload: 20, reply_to: *inbox_b.addr() }).unwrap(); + rt.send_to(actor_c, EchoMessage { payload: 30, reply_to: *inbox_c.addr() }).unwrap(); + + for _ in 0..10 { + rt.tick(); + } + + assert_eq!(inbox_a.try_recv(), Some(EchoResponse(10))); + assert_eq!(inbox_b.try_recv(), Some(EchoResponse(20))); + assert_eq!(inbox_c.try_recv(), Some(EchoResponse(30))); + // No cross-contamination + assert_eq!(inbox_a.try_recv(), None); + assert_eq!(inbox_b.try_recv(), None); + assert_eq!(inbox_c.try_recv(), None); +} + +#[test] +fn round_robin_distributes_across_workers() { + let config = RuntimeConfig { + num_threads: 3, + ..Default::default() + }; + let rt = Runtime::new(config); + + // Counter that just counts messages + struct Noop; + impl ActorInterface for Noop { + type Incoming = (); + type Response = (); + fn handle(&mut self, _ctx: &Ctx, _msg: ()) {} + } + + for _ in 0..6 { + rt.spawn(Noop).unwrap(); + } + + let s = rt.stats(); + assert_eq!(s.num_workers, 3); + // Count actors per worker from the address map snapshot + let mut per_worker = [0usize; 3]; + for (_addr, wid) in &s.actors { + per_worker[*wid] += 1; + } + // Round-robin should place exactly 2 actors on each of the 3 workers + for (wid, &count) in per_worker.iter().enumerate() { + assert_eq!(count, 2, "worker {} should have 2 actors", wid); + } +} diff --git a/tests/stats_demo.rs b/tests/stats_api.rs similarity index 51% rename from tests/stats_demo.rs rename to tests/stats_api.rs index 49901a0..7b11822 100644 --- a/tests/stats_demo.rs +++ b/tests/stats_api.rs @@ -22,7 +22,6 @@ impl ActorInterface for PingActor { } } -/// Counter that just counts messages. struct Counter(u64); impl ActorInterface for Counter { @@ -35,63 +34,43 @@ impl ActorInterface for Counter { } #[test] -fn stats_demo_single_thread() { +fn stats_reflect_actor_lifecycle() { let rt = Runtime::new(RuntimeConfig::default()); - // Spawn a few actors - let ping1 = rt.spawn(PingActor).unwrap(); - let ping2 = rt.spawn(PingActor).unwrap(); + let _ping1 = rt.spawn(PingActor).unwrap(); + let _ping2 = rt.spawn(PingActor).unwrap(); let counter = rt.spawn(Counter(0)).unwrap(); - // Send some messages (they queue up before we tick) for i in 0..20u64 { rt.send_to(counter, i).unwrap(); } - // Stats BEFORE ticking — messages are in the transfer queue, not yet in mailboxes - let s = rt.stats(); - println!("=== Before any ticks ==="); - print_stats(&s); - - // Tick once — drains transfer queue into mailboxes, then processes messages - rt.tick(); - - let s = rt.stats(); - println!("\n=== After 1 tick ==="); - print_stats(&s); - - // Tick a few more times to drain remaining messages - for _ in 0..5 { + // Tick enough to fully drain all messages + for _ in 0..6 { rt.tick(); } let s = rt.stats(); - println!("\n=== After 6 ticks total ==="); - print_stats(&s); - assert_eq!(s.num_workers, 1); assert_eq!(s.actors.len(), 3); assert_eq!(s.workers[0].num_actors, 3); - // All 20 messages should be processed by now assert_eq!(s.workers[0].mailbox_depth, 0); - assert!(s.workers[0].messages_processed >= 20); + assert_eq!(s.workers[0].messages_processed, 20); } #[test] -fn stats_demo_multi_thread() { +fn stats_reflect_multi_worker_distribution() { let config = RuntimeConfig { num_threads: 3, ..Default::default() }; let rt = Runtime::new(config); - // Spawn actors — round-robin will spread them across 3 workers let mut addrs = Vec::new(); for _ in 0..6 { addrs.push(rt.spawn(Counter(0)).unwrap()); } - // Send messages to each actor for &addr in &addrs { for i in 0..10u64 { rt.send_to(addr, i).unwrap(); @@ -99,14 +78,9 @@ fn stats_demo_multi_thread() { } let handle = rt.run().unwrap(); - - // Let it process std::thread::sleep(std::time::Duration::from_millis(50)); let s = handle.runtime.stats(); - println!("\n=== Multi-threaded (3 workers, 6 actors, 60 messages) ==="); - print_stats(&s); - handle.shutdown(); handle.join(); @@ -115,22 +89,3 @@ fn stats_demo_multi_thread() { let total_processed: u64 = s.workers.iter().map(|w| w.messages_processed).sum(); assert_eq!(total_processed, 60); } - -fn print_stats(s: &swactor::runtime::RuntimeStats) { - println!( - "RuntimeStats(actors={}, workers={})", - s.actors.len(), - s.num_workers - ); - for w in &s.workers { - println!( - " Worker {}: {} actors, {} queued, {} processed", - w.id, w.num_actors, w.mailbox_depth, w.messages_processed - ); - for (addr, wid) in &s.actors { - if *wid == w.id { - println!(" - {:x?}...", &addr.0[..4]); - } - } - } -} diff --git a/tests/test_python.py b/tests/test_python.py index 07b4bb3..3c8d691 100644 --- a/tests/test_python.py +++ b/tests/test_python.py @@ -5,41 +5,32 @@ from swactor import Runtime, RuntimeConfig, ActorAddress class TestActorAddress(unittest.TestCase): - def test_repr(self): - rt = Runtime() - addr = rt.spawn(lambda ctx, msg: None) - r = repr(addr) - self.assertTrue(r.startswith("ActorAddress(")) - self.assertTrue(r.endswith(")")) - # hex string should be 64 chars (32 bytes) - hex_part = r[len("ActorAddress("):-1] - self.assertEqual(len(hex_part), 64) - - def test_hex(self): - rt = Runtime() - addr = rt.spawn(lambda ctx, msg: None) - self.assertEqual(len(addr.hex()), 64) - - def test_to_bytes(self): - rt = Runtime() - addr = rt.spawn(lambda ctx, msg: None) - self.assertEqual(len(addr.to_bytes()), 32) - - def test_equality(self): - rt = Runtime() - addr = rt.spawn(lambda ctx, msg: None) - # Same address object should be equal to itself - self.assertEqual(addr, addr) - - def test_hashable(self): + def test_address_identity_and_collections(self): + """Addresses for distinct actors are unique, hashable, and survive repr/bytes round-trips.""" rt = Runtime() addr1 = rt.spawn(lambda ctx, msg: None) addr2 = rt.spawn(lambda ctx, msg: None) + + # Distinct actors have distinct addresses + self.assertNotEqual(addr1, addr2) + # Same address equals itself + self.assertEqual(addr1, addr1) + + # Usable as dict keys / set members s = {addr1, addr2} self.assertEqual(len(s), 2) - s.add(addr1) + s.add(addr1) # duplicate is a no-op self.assertEqual(len(s), 2) + # Bytes and hex representations are well-formed + self.assertEqual(len(addr1.to_bytes()), 32) + self.assertEqual(len(addr1.hex()), 64) + + # repr round-trip is readable + r = repr(addr1) + self.assertTrue(r.startswith("ActorAddress(")) + self.assertTrue(r.endswith(")")) + class TestRuntimeConfig(unittest.TestCase): def test_defaults(self):