refactor: better tests (#18)

Still unsatisfied, but these are better than before.


Signed-off-by: Zachery Aaron Shores-Chmielewski <zacheryasc@gmail.com>

.gitignore

@@ -2,4 +2,5 @@
 tools/depgraph/target/
 node_modules/
 .vscode/
-.venv
+.venv
+__pycache__

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));
-    }
-}

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<DoubleResponse> = 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<EchoResponse> = rt.new_inbox().unwrap();
+    let inbox_b: Inbox<EchoResponse> = rt.new_inbox().unwrap();
+    let inbox_c: Inbox<EchoResponse> = 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);
+    }
+}

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 ping1 = rt.spawn(PingActor).unwrap();
+    let _ping2 = 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
+    // Tick enough to fully drain all messages
-    let s = rt.stats();
+    for _ in 0..6 {
-    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 {
         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]);
-            }
-        }
-    }
-}

tests/test_python.py

@@ -5,41 +5,32 @@ from swactor import Runtime, RuntimeConfig, ActorAddress
 
 
 class TestActorAddress(unittest.TestCase):
-    def test_repr(self):
+    def test_address_identity_and_collections(self):
-        rt = Runtime()
+        """Addresses for distinct actors are unique, hashable, and survive repr/bytes round-trips."""
-        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):
         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):