Merge pull request #7185 from dotCooCoo/fix/mcp-integration-test-temp-folders

Tests: the vectorcraft-mcp integration tests remove their temp folders

crates/mcp/tests/automation_roots.rs

@@ -5,8 +5,11 @@
 // Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
 #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
 
-use std::path::{Path, PathBuf};
+mod common;
 
+use std::path::Path;
+
+use common::TempFolder;
 use serde_json::{Value, json};
 use vectorcraft_engine::file_access::AutomationRoots;
 use vectorcraft_mcp::{Headless, Server};
@@ -14,9 +17,10 @@ use vectorcraft_mcp::{Headless, Server};
 const SVG: &str = r#"<svg xmlns="http://www.w3.org/2000/svg" width="40" height="30"><rect width="20" height="10" fill="red"/></svg>"#;
 
 /// A folder holding `in` (the read root, with `a.svg`), `out` (the write root), `outside` (with
-/// `b.svg`) and `in2`, and a server confined to `in` and `out` (either may be left out).
-fn confined(tag: &str, read: bool, write: bool) -> (Server, PathBuf) {
-    let base = vectorcraft_testkit::temp_dir(&format!("roots-{tag}"));
+/// `b.svg`) and `in2`, and a server confined to `in` and `out` (either may be left out). The
+/// folder is removed when the returned [`TempFolder`] is dropped.
+fn confined(tag: &str, read: bool, write: bool) -> (Server, TempFolder) {
+    let base = TempFolder::new(&format!("roots-{tag}"));
     for d in ["in", "out", "outside", "in2"] {
         std::fs::create_dir_all(base.join(d)).unwrap();
     }
@@ -55,6 +59,29 @@ fn refused(server: &mut Server, name: &str, args: Value, why: &str) {
     assert!(error && text.contains(why), "{name} {args} should be refused ({why}): {text}");
 }
 
+/// A `TempFolder` is removed with the files and folders in it when dropped, also when the test
+/// that holds it panics.
+#[test]
+fn temp_folders_are_removed_when_dropped() {
+    let dir = TempFolder::new("roots-drop-check");
+    std::fs::create_dir(dir.join("sub")).unwrap();
+    std::fs::write(dir.join("sub/a.svg"), SVG).unwrap();
+    let path = dir.to_path_buf();
+    drop(dir);
+    assert!(!path.exists(), "{} is removed", path.display());
+
+    let mut held = None;
+    let panicked = vectorcraft_testkit::catch_quiet(|| {
+        let dir = TempFolder::new("roots-panic-check");
+        std::fs::write(dir.join("a.svg"), SVG).unwrap();
+        held = Some(dir.to_path_buf());
+        panic!("the test fails with {} in use", dir.display());
+    });
+    assert!(panicked.is_err());
+    let path = held.unwrap();
+    assert!(!path.exists(), "{} is removed after a panic", path.display());
+}
+
 #[test]
 fn files_open_only_from_inside_the_read_root() {
     let (mut server, base) = confined("open", true, true);

crates/mcp/tests/common/mod.rs

@@ -0,0 +1,28 @@
+//! Shared helpers for the MCP integration tests.
+
+use std::path::{Path, PathBuf};
+
+/// A test's own folder, made by [`vectorcraft_testkit::temp_dir`] (with its checks against a
+/// folder made by someone else) and removed with everything in it when dropped, also when the
+/// test panics. A removal that fails (a file still open) is ignored. Tests that run at the same
+/// time use different tags.
+pub struct TempFolder(PathBuf);
+
+impl TempFolder {
+    pub fn new(tag: &str) -> Self {
+        Self(vectorcraft_testkit::temp_dir(tag))
+    }
+}
+
+impl std::ops::Deref for TempFolder {
+    type Target = Path;
+    fn deref(&self) -> &Path {
+        &self.0
+    }
+}
+
+impl Drop for TempFolder {
+    fn drop(&mut self) {
+        let _ = std::fs::remove_dir_all(&self.0);
+    }
+}

crates/mcp/tests/protocol_props.rs

@@ -4,6 +4,9 @@
 // Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
 #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
 
+mod common;
+
+use common::TempFolder;
 use proptest::prelude::*;
 use serde_json::{Value, json};
 use vectorcraft_doc::Document;
@@ -98,7 +101,7 @@ fn tool_definitions_are_valid() {
 
 #[test]
 fn every_tool_survives_empty_and_junk_arguments() {
-    let dir = vectorcraft_testkit::temp_dir("mcp-junk");
+    let dir = TempFolder::new("mcp-junk");
     let safe = dir.join("junk.vectorcraft").to_string_lossy().to_string();
     let mut failures = vec![];
     for t in tool_definitions() {
@@ -169,7 +172,7 @@ proptest! {
         tail in junk_tail(),
         tool in prop::sample::select(vec!["open_file", "save_file", "export", "screenshot"]),
     ) {
-        let base = vectorcraft_testkit::temp_dir("mcp-confined");
+        let base = TempFolder::new("mcp-confined");
         let root = base.join("a/b/c/root");
         std::fs::create_dir_all(&root).unwrap();
         std::fs::create_dir_all(base.join("outside")).unwrap();