Merge pull request #18 from tinyhumansai/tests-to-files

docs: require unit tests in *_tests.rs files

Author: senamakel

AGENTS.md

@@ -48,15 +48,17 @@ placeholder.
 Each feature area is a directory module under a crate's `src/`. A module root
 explains the module, wires its pieces together, and exposes the smallest useful
 API. Move substantial type definitions into `types.rs` and put module-local
-unit tests in a dedicated `test.rs`, wired from the bottom of the module root
+unit tests in a sibling `<module>_tests.rs`, wired from the bottom of the module root
 with:
 
 ```rust
 #[cfg(test)]
-mod test;
+#[path = "mod_tests.rs"]
+mod tests;
 ```
 
-Do not accumulate inline `mod tests` blocks in implementation files, and do not
+Do not write inline `mod tests` blocks in implementation files, do not name a test
+file `test.rs`, `tests.rs` or `<module>_test.rs`, and do not
 let a general-purpose `utils.rs` or `helpers.rs` grow — those are a symptom of a
 missing module. Prefer many small modules that each do one thing well over few
 broad ones.
@@ -186,7 +188,7 @@ new module capability requires more.
 
 ## Testing
 
-- Module-local unit tests live in `crates/<crate>/src/<feature>/test.rs` and may
+- Module-local unit tests live in `crates/<crate>/src/<feature>/mod_tests.rs` and may
   touch private items.
 - `unwrap_used`, `expect_used`, and `panic` are denied in **test** targets too.
   Return `Result` from a test and use `?` for paths that should succeed; assert
@@ -218,7 +220,7 @@ Write documentation for the reader who has never seen the code.
 
 - Every public item gets a rustdoc comment. `missing_docs` is a warning that CI
   treats as an error.
-- Start every `mod.rs` and `test.rs` with a concise module-level `//!`
+- Start every `mod.rs` and `*_tests.rs` with a concise module-level `//!`
   description.
 - `src/lib.rs` carries the crate-level overview: what the crate does, the
   primary entry points, and a short runnable example.
@@ -307,3 +309,28 @@ For automated contributors specifically:
    credentials, and never paste them into a pull request or issue.
 7. **Ask only when blocked.** Make routine judgment calls yourself; escalate
    only irreversible decisions or genuine forks with no clear default.
+
+## 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.
+- Existing `test.rs` and `<module>_test.rs` files predate this rule. Rename each
+  to `<module>_tests.rs` (keep its `mod` name, add the `#[path]` attribute) the
+  next time you touch it.

crates/tinybox-cli/src/command/test.rs

@@ -89,7 +89,7 @@ async fn a_box_can_be_created_used_and_removed() -> Result<()> {
     let removed = invoke(dir.path(), &["rm", "box-0"]).await;
     assert_eq!(removed.code, 0);
 
-    assert!(invoke(dir.path(), &["ls"]).await.out.is_empty());
+    assert_eq!(invoke(dir.path(), &["ls"]).await.out.len(), 0);
     Ok(())
 }
 
@@ -197,7 +197,7 @@ async fn run_creates_uses_and_destroys_a_box_in_one_step() -> Result<()> {
     assert_eq!(executed.code, 0);
     assert_eq!(executed.out.trim(), "once");
     // Nothing is left behind.
-    assert!(invoke(dir.path(), &["ls"]).await.out.is_empty());
+    assert_eq!(invoke(dir.path(), &["ls"]).await.out.len(), 0);
     Ok(())
 }
 
@@ -209,7 +209,7 @@ async fn run_leaves_nothing_behind_when_the_command_fails() -> Result<()> {
 
     assert_eq!(executed.code, 3);
     // A failing command must not leak a box; the cleanup is unconditional.
-    assert!(invoke(dir.path(), &["ls"]).await.out.is_empty());
+    assert_eq!(invoke(dir.path(), &["ls"]).await.out.len(), 0);
     Ok(())
 }
 
@@ -225,7 +225,7 @@ async fn run_reports_a_command_that_cannot_start_and_still_cleans_up() -> Result
 
     assert_eq!(executed.code, EXIT_TINYBOX_ERROR);
     assert!(executed.err.contains("error:"));
-    assert!(invoke(dir.path(), &["ls"]).await.out.is_empty());
+    assert_eq!(invoke(dir.path(), &["ls"]).await.out.len(), 0);
     Ok(())
 }
 
@@ -313,7 +313,7 @@ async fn a_usage_error_reports_clap_s_exit_code() -> Result<()> {
     let outcome = invoke(dir.path(), &["not-a-command"]).await;
 
     assert_eq!(outcome.code, 2);
-    assert!(!outcome.err.is_empty());
+    assert_ne!(outcome.err.len(), 0);
     Ok(())
 }
 
@@ -343,8 +343,8 @@ async fn requested_help_goes_to_stdout_and_usage_errors_to_stderr() -> Result<()
     // A mistake is a diagnostic, and stays on stderr.
     let misuse = invoke(dir.path(), &["not-a-command"]).await;
     assert_eq!(misuse.code, 2);
-    assert!(misuse.out.is_empty());
-    assert!(!misuse.err.is_empty());
+    assert_eq!(misuse.out.len(), 0);
+    assert_ne!(misuse.err.len(), 0);
     Ok(())
 }
 
@@ -848,11 +848,12 @@ async fn a_one_shot_docker_run_leaves_nothing_behind() -> Result<()> {
 
     assert_eq!(executed.code, 0);
     assert_eq!(executed.out.trim(), "once");
-    assert!(
+    assert_eq!(
         invoke_scripted(dir.path(), host.clone(), &["ls"])
             .await
             .out
-            .is_empty()
+            .len(),
+        0
     );
     Ok(())
 }
@@ -932,7 +933,7 @@ async fn an_ssh_destination_that_would_be_read_as_an_option_is_refused() -> Resu
 
     assert_eq!(outcome.code, EXIT_TINYBOX_ERROR);
     assert!(outcome.err.contains("ssh destination"));
-    assert!(host.commands().is_empty());
+    assert_eq!(host.commands().len(), 0);
     Ok(())
 }
 
@@ -1151,7 +1152,7 @@ async fn templates_can_be_listed_and_forgotten() -> Result<()> {
     assert!(listed.out.contains("sha-9f2c0e1b7a4d"));
 
     assert_eq!(invoke(dir.path(), &["template", "rm", "ci"]).await.code, 0);
-    assert!(invoke(dir.path(), &["template", "ls"]).await.out.is_empty());
+    assert_eq!(invoke(dir.path(), &["template", "ls"]).await.out.len(), 0);
     Ok(())
 }
 

crates/tinybox-cli/src/store/test.rs

@@ -38,7 +38,7 @@ fn a_missing_file_reads_as_an_empty_store() -> Result<()> {
 
     // No initialization step: a fresh install must just work.
     assert!(!store.path().exists());
-    assert!(store.list()?.is_empty());
+    assert_eq!(store.list()?.len(), 0);
     assert_eq!(store.allocate_id()?.as_str(), "box-0");
     Ok(())
 }
@@ -117,7 +117,7 @@ fn state_changes_and_removals_persist() -> Result<()> {
     );
 
     store.remove(&recorded.id)?;
-    assert!(FileStore::new(store.path()).list()?.is_empty());
+    assert_eq!(FileStore::new(store.path()).list()?.len(), 0);
     Ok(())
 }
 

crates/tinybox-cli/src/templates/test.rs

@@ -30,7 +30,7 @@ fn a_missing_file_reads_as_an_empty_index() -> Result<()> {
     let (templates, _dir) = templates()?;
 
     assert!(!templates.path().exists());
-    assert!(templates.list()?.is_empty());
+    assert_eq!(templates.list()?.len(), 0);
     Ok(())
 }
 
@@ -73,7 +73,7 @@ fn removals_persist() -> Result<()> {
 
     templates.remove(&name("ci")?)?;
 
-    assert!(FileTemplates::new(templates.path()).list()?.is_empty());
+    assert_eq!(FileTemplates::new(templates.path()).list()?.len(), 0);
     Ok(())
 }
 

crates/tinybox-cli/tests/binary.rs

@@ -56,7 +56,7 @@ fn a_box_is_created_used_and_destroyed_through_the_binary() -> io::Result<()> {
 
     let removed = tinybox(state.path(), &["rm", "box-0"])?;
     assert!(removed.status.success());
-    assert!(stdout(&tinybox(state.path(), &["ls"])?).is_empty());
+    assert_eq!(stdout(&tinybox(state.path(), &["ls"])?).len(), 0);
     Ok(())
 }
 

crates/tinybox-core/src/capability/test.rs

@@ -41,7 +41,7 @@ fn passthrough_admits_it_isolates_nothing() {
 fn each_builder_method_adds_exactly_one_capability() {
     let base = SandboxCapabilities::new(IsolationLevel::Kernel, SnapshotSupport::None);
 
-    assert!(base.declared().is_empty());
+    assert_eq!(base.declared().len(), 0);
     assert_eq!(base.with_fork().declared(), [Capability::Fork]);
     assert_eq!(
         base.with_pause_resume().declared(),
@@ -104,7 +104,7 @@ fn a_declared_set_lists_snapshot_and_feature_capabilities_together() {
         ]
     );
     assert_eq!(MICROVM.declared(), Capability::ALL);
-    assert!(SandboxCapabilities::PASSTHROUGH.declared().is_empty());
+    assert_eq!(SandboxCapabilities::PASSTHROUGH.declared().len(), 0);
 }
 
 #[test]

crates/tinybox-core/src/shell/scan_test.rs

@@ -53,8 +53,8 @@ fn single_ampersand_stays_inside_its_segment() {
 
 #[test]
 fn empty_and_blank_segments_are_dropped() {
-    assert!(segs("").is_empty());
-    assert!(segs("  ;  ;\n").is_empty());
+    assert_eq!(segs("").len(), 0);
+    assert_eq!(segs("  ;  ;\n").len(), 0);
     assert_eq!(segs(";;a;;"), ["a"]);
 }
 

crates/tinybox-core/src/store/test.rs

@@ -79,7 +79,7 @@ fn removing_a_box_forgets_it() -> Result<()> {
 
     store.remove(&recorded.id)?;
 
-    assert!(store.list()?.is_empty());
+    assert_eq!(store.list()?.len(), 0);
     assert!(store.get(&recorded.id).is_err());
     Ok(())
 }
@@ -140,7 +140,7 @@ fn allocation_steps_past_identifiers_that_are_not_generated_names() -> Result<()
 
 #[test]
 fn a_default_store_is_empty() -> Result<()> {
-    assert!(MemoryStore::default().list()?.is_empty());
+    assert_eq!(MemoryStore::default().list()?.len(), 0);
     Ok(())
 }
 
@@ -304,7 +304,7 @@ fn a_failure_that_is_not_a_collision_is_not_retried() -> Result<()> {
     // The rest of the stub behaves as the trait requires, so a future change
     // that starts calling it does not silently get nonsense.
     let id = BoxId::new("box-0")?;
-    assert!(ReadOnly.list()?.is_empty());
+    assert_eq!(ReadOnly.list()?.len(), 0);
     assert!(ReadOnly.get(&id).is_err());
     assert!(ReadOnly.set_state(&id, BoxState::Stopped).is_ok());
     assert!(ReadOnly.remove(&id).is_ok());

crates/tinybox-core/src/template/test.rs

@@ -102,7 +102,7 @@ fn a_template_name_follows_the_usual_identifier_rule() {
 
 #[test]
 fn an_empty_index_lists_nothing() -> Result<()> {
-    assert!(MemoryTemplates::default().list()?.is_empty());
+    assert_eq!(MemoryTemplates::default().list()?.len(), 0);
     Ok(())
 }
 

crates/tinybox-docker/src/oneshot/test.rs

@@ -200,7 +200,7 @@ async fn run_times_out_with_the_pinned_message() {
         .unwrap();
     assert!(got.timed_out);
     assert_eq!(got.exit_code, -1);
-    assert!(got.stdout.is_empty());
+    assert_eq!(got.stdout.len(), 0);
     assert_eq!(got.stderr, "Command timed out after 0s and was killed");
 }