test: move inline tests into *_tests.rs files

AGENTS.md

@@ -29,3 +29,25 @@ Make small commits as much as possible, keeping each commit focused on one coher
 ## Parallel Work
 
 Use git worktrees when running tasks in parallel or when launching sub-agents in parallel, so each concurrent effort has an isolated checkout and does not disturb another worktree's files, build output, or branch state.
+
+## Tests live in `*_tests.rs` files
+
+- Unit tests are never inline. Do not write a `#[cfg(test)] mod tests { ... }`
+  block in a source file. Put the tests in a sibling `<module>_tests.rs`
+  (`mod_tests.rs` beside a `mod.rs`, `lib_tests.rs` beside `lib.rs`) and declare
+  it at the bottom of the module:
+
+  ```rust
+  #[cfg(test)]
+  #[path = "foo_tests.rs"]
+  mod tests;
+  ```
+
+- The test file starts with `use super::*;` and carries no `#[cfg(test)]` of its
+  own. It is still a child module, so it reaches private items exactly as an
+  inline module did.
+- Name test files `<module>_tests.rs`; a second group for the same module is
+  `<module>_<topic>_tests.rs`. Never `test.rs`, `tests.rs` or `<module>_test.rs`.
+- Integration tests stay in the crate's `tests/` directory.
+- OpenHuman's `scripts/externalize-inline-tests.mjs <repo-root> --write` moves
+  inline test modules out mechanically; without `--write` it only reports.

src/memory/graph/bfs.rs

@@ -81,38 +81,5 @@ pub fn pair_distances(
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::memory::graph::{pairs_from_entities, upsert_edges};
-
-    #[test]
-    fn bounded_bfs_finds_two_hop_pair() {
-        let temp = tempfile::tempdir().unwrap();
-        let config = MemoryConfig::new(temp.path());
-        upsert_edges(
-            &config,
-            &pairs_from_entities(&["alice".into(), "bob".into()]),
-            1,
-        )
-        .unwrap();
-        upsert_edges(
-            &config,
-            &pairs_from_entities(&["bob".into(), "carol".into()]),
-            1,
-        )
-        .unwrap();
-        assert!(
-            pair_distances(&config, &["alice".into(), "carol".into()], 1)
-                .unwrap()
-                .is_empty()
-        );
-        assert_eq!(
-            pair_distances(&config, &["alice".into(), "carol".into()], 2).unwrap(),
-            vec![PairDistance {
-                a: "alice".into(),
-                b: "carol".into(),
-                dist: 2,
-            }]
-        );
-    }
-}
+#[path = "bfs_tests.rs"]
+mod tests;

src/memory/graph/bfs_tests.rs

@@ -0,0 +1,33 @@
+use super::*;
+use crate::memory::graph::{pairs_from_entities, upsert_edges};
+
+#[test]
+fn bounded_bfs_finds_two_hop_pair() {
+    let temp = tempfile::tempdir().unwrap();
+    let config = MemoryConfig::new(temp.path());
+    upsert_edges(
+        &config,
+        &pairs_from_entities(&["alice".into(), "bob".into()]),
+        1,
+    )
+    .unwrap();
+    upsert_edges(
+        &config,
+        &pairs_from_entities(&["bob".into(), "carol".into()]),
+        1,
+    )
+    .unwrap();
+    assert!(
+        pair_distances(&config, &["alice".into(), "carol".into()], 1)
+            .unwrap()
+            .is_empty()
+    );
+    assert_eq!(
+        pair_distances(&config, &["alice".into(), "carol".into()], 2).unwrap(),
+        vec![PairDistance {
+            a: "alice".into(),
+            b: "carol".into(),
+            dist: 2,
+        }]
+    );
+}

src/memory/graph/edge_store.rs

@@ -106,25 +106,5 @@ pub fn count_edges(config: &MemoryConfig) -> Result<u64> {
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn persisted_edges_are_canonical_weighted_and_symmetric() {
-        let temp = tempfile::tempdir().unwrap();
-        let config = MemoryConfig::new(temp.path());
-        let pairs = pairs_from_entities(&[
-            "person:bob".into(),
-            "person:alice".into(),
-            "person:alice".into(),
-        ]);
-        assert_eq!(pairs, vec![("person:alice".into(), "person:bob".into())]);
-        upsert_edges(&config, &pairs, 1).unwrap();
-        upsert_edges(&config, &pairs, 2).unwrap();
-        assert_eq!(
-            edge_neighbors(&config, "person:bob").unwrap(),
-            vec![("person:alice".into(), 2)]
-        );
-        assert_eq!(count_edges(&config).unwrap(), 1);
-    }
-}
+#[path = "edge_store_tests.rs"]
+mod tests;

src/memory/graph/edge_store_tests.rs

@@ -0,0 +1,20 @@
+use super::*;
+
+#[test]
+fn persisted_edges_are_canonical_weighted_and_symmetric() {
+    let temp = tempfile::tempdir().unwrap();
+    let config = MemoryConfig::new(temp.path());
+    let pairs = pairs_from_entities(&[
+        "person:bob".into(),
+        "person:alice".into(),
+        "person:alice".into(),
+    ]);
+    assert_eq!(pairs, vec![("person:alice".into(), "person:bob".into())]);
+    upsert_edges(&config, &pairs, 1).unwrap();
+    upsert_edges(&config, &pairs, 2).unwrap();
+    assert_eq!(
+        edge_neighbors(&config, "person:bob").unwrap(),
+        vec![("person:alice".into(), 2)]
+    );
+    assert_eq!(count_edges(&config).unwrap(), 1);
+}

src/memory/people/address_book.rs

@@ -291,92 +291,5 @@ mod imp {
 // ── tests ─────────────────────────────────────────────────────────────────────
 
 #[cfg(test)]
-pub mod tests {
-    use super::*;
-
-    /// Test double that returns a canned list without any FFI calls.
-    pub struct MockContactsSource {
-        pub result: Result<Vec<AddressBookContact>, AddressBookError>,
-    }
-
-    impl MockContactsSource {
-        pub fn ok(contacts: Vec<AddressBookContact>) -> Self {
-            Self {
-                result: Ok(contacts),
-            }
-        }
-
-        pub fn permission_denied() -> Self {
-            Self {
-                result: Err(AddressBookError::PermissionDenied),
-            }
-        }
-    }
-
-    impl ContactsSource for MockContactsSource {
-        fn fetch_contacts(&self) -> Result<Vec<AddressBookContact>, AddressBookError> {
-            match &self.result {
-                Ok(v) => Ok(v.clone()),
-                Err(AddressBookError::PermissionDenied) => Err(AddressBookError::PermissionDenied),
-                Err(AddressBookError::Other(s)) => Err(AddressBookError::Other(s.clone())),
-            }
-        }
-    }
-
-    fn mk_contact(name: &str, email: &str) -> AddressBookContact {
-        AddressBookContact {
-            display_name: Some(name.into()),
-            emails: vec![email.into()],
-            phones: vec![],
-        }
-    }
-
-    #[test]
-    fn mock_source_returns_canned_contacts() {
-        let source = MockContactsSource::ok(vec![
-            mk_contact("Alice", "alice@example.com"),
-            mk_contact("Bob", "bob@example.com"),
-        ]);
-        let result = read_with(&source).unwrap();
-        assert_eq!(result.len(), 2);
-        assert_eq!(result[0].display_name.as_deref(), Some("Alice"));
-        assert_eq!(result[1].emails[0], "bob@example.com");
-    }
-
-    #[test]
-    fn mock_source_permission_denied_is_distinguished() {
-        let source = MockContactsSource::permission_denied();
-        let err = read_with(&source).unwrap_err();
-        assert_eq!(err, AddressBookError::PermissionDenied);
-    }
-
-    #[test]
-    fn system_source_non_mac_returns_empty() {
-        // Mirrors the `imp` cfgs above: the stub is what compiles whenever the
-        // real CNContactStore path is absent, whether by target or by gate.
-        #[cfg(not(all(target_os = "macos", feature = "contacts")))]
-        {
-            let source = SystemContactsSource;
-            let result = read_with(&source).unwrap();
-            assert!(result.is_empty());
-        }
-        #[cfg(all(target_os = "macos", feature = "contacts"))]
-        {
-            // TCC state is environment-dependent; just verify no panic.
-            let source = SystemContactsSource;
-            let _ = read_with(&source);
-        }
-    }
-
-    #[test]
-    fn contact_with_no_fields_is_excluded_by_mock() {
-        let source = MockContactsSource::ok(vec![AddressBookContact {
-            display_name: Some("Sarah Lee".into()),
-            emails: vec![],
-            phones: vec!["+1 555 000 0001".into()],
-        }]);
-        let result = read_with(&source).unwrap();
-        assert_eq!(result.len(), 1);
-        assert_eq!(result[0].phones[0], "+1 555 000 0001");
-    }
-}
+#[path = "address_book_tests.rs"]
+pub mod tests;

src/memory/people/address_book_tests.rs

@@ -0,0 +1,87 @@
+use super::*;
+
+/// Test double that returns a canned list without any FFI calls.
+pub struct MockContactsSource {
+    pub result: Result<Vec<AddressBookContact>, AddressBookError>,
+}
+
+impl MockContactsSource {
+    pub fn ok(contacts: Vec<AddressBookContact>) -> Self {
+        Self {
+            result: Ok(contacts),
+        }
+    }
+
+    pub fn permission_denied() -> Self {
+        Self {
+            result: Err(AddressBookError::PermissionDenied),
+        }
+    }
+}
+
+impl ContactsSource for MockContactsSource {
+    fn fetch_contacts(&self) -> Result<Vec<AddressBookContact>, AddressBookError> {
+        match &self.result {
+            Ok(v) => Ok(v.clone()),
+            Err(AddressBookError::PermissionDenied) => Err(AddressBookError::PermissionDenied),
+            Err(AddressBookError::Other(s)) => Err(AddressBookError::Other(s.clone())),
+        }
+    }
+}
+
+fn mk_contact(name: &str, email: &str) -> AddressBookContact {
+    AddressBookContact {
+        display_name: Some(name.into()),
+        emails: vec![email.into()],
+        phones: vec![],
+    }
+}
+
+#[test]
+fn mock_source_returns_canned_contacts() {
+    let source = MockContactsSource::ok(vec![
+        mk_contact("Alice", "alice@example.com"),
+        mk_contact("Bob", "bob@example.com"),
+    ]);
+    let result = read_with(&source).unwrap();
+    assert_eq!(result.len(), 2);
+    assert_eq!(result[0].display_name.as_deref(), Some("Alice"));
+    assert_eq!(result[1].emails[0], "bob@example.com");
+}
+
+#[test]
+fn mock_source_permission_denied_is_distinguished() {
+    let source = MockContactsSource::permission_denied();
+    let err = read_with(&source).unwrap_err();
+    assert_eq!(err, AddressBookError::PermissionDenied);
+}
+
+#[test]
+fn system_source_non_mac_returns_empty() {
+    // Mirrors the `imp` cfgs above: the stub is what compiles whenever the
+    // real CNContactStore path is absent, whether by target or by gate.
+    #[cfg(not(all(target_os = "macos", feature = "contacts")))]
+    {
+        let source = SystemContactsSource;
+        let result = read_with(&source).unwrap();
+        assert!(result.is_empty());
+    }
+    #[cfg(all(target_os = "macos", feature = "contacts"))]
+    {
+        // TCC state is environment-dependent; just verify no panic.
+        let source = SystemContactsSource;
+        let _ = read_with(&source);
+    }
+}
+
+#[test]
+fn contact_with_no_fields_is_excluded_by_mock() {
+    let source = MockContactsSource::ok(vec![AddressBookContact {
+        display_name: Some("Sarah Lee".into()),
+        emails: vec![],
+        phones: vec!["+1 555 000 0001".into()],
+    }]);
+    let result = read_with(&source).unwrap();
+    assert_eq!(result.len(), 1);
+    assert_eq!(result[0].phones[0], "+1 555 000 0001");
+}

src/memory/people/migrations.rs

@@ -46,48 +46,5 @@ pub fn run(conn: &Connection) -> Result<()> {
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-
-    fn fresh() -> Connection {
-        Connection::open_in_memory().unwrap()
-    }
-
-    #[test]
-    fn migrations_create_expected_tables() {
-        let conn = fresh();
-        run(&conn).unwrap();
-        let mut stmt = conn
-            .prepare("SELECT name FROM sqlite_master WHERE type='table' ORDER BY name")
-            .unwrap();
-        let names: Vec<String> = stmt
-            .query_map([], |row| row.get(0))
-            .unwrap()
-            .map(|r| r.unwrap())
-            .collect();
-        for expected in [
-            "people",
-            "handle_aliases",
-            "interactions",
-            "_people_migrations",
-        ] {
-            assert!(
-                names.iter().any(|n| n == expected),
-                "missing {expected}: {names:?}"
-            );
-        }
-    }
-
-    #[test]
-    fn migrations_are_idempotent() {
-        let conn = fresh();
-        run(&conn).unwrap();
-        run(&conn).unwrap();
-        let count: i64 = conn
-            .query_row("SELECT count(*) FROM _people_migrations", [], |row| {
-                row.get(0)
-            })
-            .unwrap();
-        assert_eq!(count, MIGRATIONS.len() as i64);
-    }
-}
+#[path = "migrations_tests.rs"]
+mod tests;