From 940b47234b6a6c64b6be85318af466179e283a44 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 08:22:23 -0800 Subject: [PATCH] Tests: the vectorcraft-mcp integration tests remove their temp folders automation_roots.rs and protocol_props.rs wrote their files into folders made by vectorcraft_testkit::temp_dir (vectorcraft-testkit-roots--, mcp-junk, mcp-confined), and nothing removed them after the run. A shared tests/common module adds TempFolder, which takes the folder from temp_dir, with its checks against a folder made by another user, and removes it with everything in it when dropped, also when the test panics, as the findfiles tests' Tree guard does (crates/engine/src/cmd/findfiles/tests.rs:49). Each proptest case of a_confined_server_writes_only_inside_its_root now gets a new folder. A new test checks that the folder is removed on drop and after a panic. --- crates/mcp/tests/automation_roots.rs | 35 ++++++++++++++++++++++++---- crates/mcp/tests/common/mod.rs | 28 ++++++++++++++++++++++ crates/mcp/tests/protocol_props.rs | 7 ++++-- 3 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 crates/mcp/tests/common/mod.rs diff --git a/crates/mcp/tests/automation_roots.rs b/crates/mcp/tests/automation_roots.rs index d45ecc7142..74bd95d455 100644 --- a/crates/mcp/tests/automation_roots.rs +++ b/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#""#; /// 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); diff --git a/crates/mcp/tests/common/mod.rs b/crates/mcp/tests/common/mod.rs new file mode 100644 index 0000000000..08954ca21e --- /dev/null +++ b/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); + } +} diff --git a/crates/mcp/tests/protocol_props.rs b/crates/mcp/tests/protocol_props.rs index f9dc065ff1..b3145a20b9 100644 --- a/crates/mcp/tests/protocol_props.rs +++ b/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();