mirror of
https://github.com/block/buzz.git
synced 2026-11-01 06:51:17 +00:00
test: stop desktop, buzz-agent and mobile tests leaking temp files (#8748)
## Summary Six test files wrote into the shared temp dir on every normal run and never removed what they wrote. **Desktop and buzz-agent (first commit)** - `desktop/src/protectedFeatures/tauriCommand.test.mjs` wrote its fake Tauri CLI and three result files straight into `/tmp` under pid-suffixed names. They now live under one `mkdtempSync` root that an `after()` hook removes. - `crates/buzz-agent/tests/permission_boundary.rs` wrote a `buzz_perm_calllog_*` file for every test whose tool call reached `fake_mcp`, five per run. `call_log_path` now returns a `TempDir` guard with the log path inside it, so the log is deleted when the test ends, including when an assertion panics. - `cancel_kills_inflight_tool_via_mcp_notification` in `crates/buzz-agent/tests/regressions.rs` spawns `buzz-dev-mcp`, which the agent stops with SIGKILL, so the server's `buzz-dev-mcp-*` and `buzz-dev-mcp-session-*` TempDirs were never deleted. The test now passes the server a `TMPDIR` it owns and removes. **Mobile (second commit)** The three mobile test files left 12 `buzz-video-*` files and 4 `transcode_*` dirs per run. - `video_viewer_test.dart` and `video_controls_test.dart` pointed their fake `path_provider` at `Directory.systemTemp`. The viewer downloads each video there as `buzz-video-*`, and its async delete on close never completes under FakeAsync. Each test now gets its own temp dir for the fake, and an `addTearDown` removes it with `deleteSync`. In `video_controls_test.dart` that teardown is registered first, so it runs after the viewer is disposed. - Four `media_upload_test.dart` tests faked `transcodeVideoToMp4` by writing `out.mp4` into a new `systemTemp` `transcode_*` dir. The upload service deletes `out.mp4` but not its dir. The fakes now create that dir under the test's own temp root, which the test already deletes in `finally`. Production code is unchanged. **`buzz-dev-mcp`'s SIGKILL shutdown is left for a follow-up:** the agent still stops the server with SIGKILL, so outside tests the server's temp dirs are still left behind. Sending SIGTERM first is the real fix. ### Related issue None found. This continues the test-cleanup work in #8727. ### Testing Each run below was bracketed by listings of `/tmp` and the per-user temp dir (`/var/folders/.../T`), so new leftovers would show up. - `tauriCommand.test.mjs`: 3/3 pass, with no new `buzz-tauri-*` or `buzz-fake-tauri-*` entries. - `cargo test -p buzz-agent --test permission_boundary`: 9/9 pass, with no new `buzz_perm_calllog_*` files. The tests still check that the call log is written, so the log really is going into the guarded dir. - `cancel_kills_inflight_tool_via_mcp_notification` passes with `buzz-dev-mcp` built, so it ran rather than skipped, and left no new `buzz-dev-mcp-*` dirs. - Mobile: `video_viewer_test` (7 tests), `video_controls_test` (20) and `media_upload_test` (38) pass, each run on its own. With `TMPDIR` pointed at a scratch dir, the old versions left 12 `buzz-video-*` files and 4 `transcode_*` dirs, and the new versions left it empty. - After cherry-picking onto current main, these pass: - `cargo clippy -p buzz-agent --all-targets -- -D warnings` - biome and `node --check` on `tauriCommand.test.mjs` - `dart format` and `flutter analyze` on the three mobile files 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -136,18 +136,16 @@ async fn drive(
|
||||
// Session init + call-log helpers
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
fn call_log_path(tag: &str) -> String {
|
||||
let p = std::env::temp_dir().join(format!(
|
||||
"buzz_perm_calllog_{tag}_{}_{:x}.log",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.unwrap_or_default()
|
||||
.as_nanos()
|
||||
));
|
||||
let s = p.to_string_lossy().to_string();
|
||||
let _ = std::fs::remove_file(&s);
|
||||
s
|
||||
/// A fresh call-log path inside its own temp dir. Keep the returned `TempDir`
|
||||
/// bound for the whole test: dropping it deletes the log, including when an
|
||||
/// assertion panics.
|
||||
fn call_log_path(tag: &str) -> (tempfile::TempDir, String) {
|
||||
let dir = tempfile::Builder::new()
|
||||
.prefix(&format!("buzz_perm_calllog_{tag}_"))
|
||||
.tempdir()
|
||||
.expect("create call-log temp dir");
|
||||
let path = dir.path().join("calls.log").to_string_lossy().into_owned();
|
||||
(dir, path)
|
||||
}
|
||||
|
||||
/// Invoked tool names recorded by fake_mcp (bare names, one per `tools/call`).
|
||||
@@ -214,7 +212,7 @@ fn stop_reason(resp: &Value) -> String {
|
||||
/// the tally ends at one.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_allow_reaches_tool_exactly_once_and_not_before_approval() {
|
||||
let log = call_log_path("allow");
|
||||
let (_log_dir, log) = call_log_path("allow");
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("done")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
@@ -250,7 +248,7 @@ async fn test_allow_reaches_tool_exactly_once_and_not_before_approval() {
|
||||
/// sees a permission-denied tool error (the turn continues to `end_turn`).
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_reject_option_never_invokes_tool() {
|
||||
let log = call_log_path("reject");
|
||||
let (_log_dir, log) = call_log_path("reject");
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("understood")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
@@ -321,7 +319,7 @@ async fn test_adversarial_outcomes_never_invoke_tool() {
|
||||
];
|
||||
|
||||
for (tag, build) in shapes {
|
||||
let log = call_log_path(tag);
|
||||
let (_log_dir, log) = call_log_path(tag);
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("ok")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
@@ -352,7 +350,7 @@ async fn test_adversarial_outcomes_never_invoke_tool() {
|
||||
/// waiter; the correct id then authorizes exactly once.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_stale_id_ignored_then_real_id_authorizes() {
|
||||
let log = call_log_path("staleid");
|
||||
let (_log_dir, log) = call_log_path("staleid");
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("done")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
@@ -406,7 +404,7 @@ async fn test_stale_id_ignored_then_real_id_authorizes() {
|
||||
/// by staying silent, and cancellation must not depend on that answer.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_cancel_while_waiting_executes_nothing_without_client_answer() {
|
||||
let log = call_log_path("cancelwait");
|
||||
let (_log_dir, log) = call_log_path("cancelwait");
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("done")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
@@ -452,7 +450,7 @@ async fn test_cancel_while_waiting_executes_nothing_without_client_answer() {
|
||||
/// proves only the allowed (second) call reached MCP — order-agnostic.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_crossed_parallel_decisions_authorize_only_matching_call() {
|
||||
let log = call_log_path("crossed");
|
||||
let (_log_dir, log) = call_log_path("crossed");
|
||||
// Two distinct registered tools so the call log distinguishes them by name.
|
||||
let llm = spawn_capturing_llm(vec![
|
||||
openai_tool_calls(&[
|
||||
@@ -547,7 +545,7 @@ async fn test_load_skill_emits_no_permission_request() {
|
||||
/// objects once, forcing a hook invocation the test can observe.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_stop_hook_reaches_mcp_without_permission_ask() {
|
||||
let log = call_log_path("stophook");
|
||||
let (_log_dir, log) = call_log_path("stophook");
|
||||
// Text turn triggers the _Stop gate → hook objects once → agent loops →
|
||||
// second text turn, hook silent → end_turn. No model-issued tool call.
|
||||
let llm = spawn_capturing_llm(vec![
|
||||
@@ -599,7 +597,7 @@ async fn test_stop_hook_reaches_mcp_without_permission_ask() {
|
||||
/// with an end-to-end proof that the negotiated version reaches the wire.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn test_v2_negotiation_emits_v2_request_shape() {
|
||||
let log = call_log_path("v2shape");
|
||||
let (_log_dir, log) = call_log_path("v2shape");
|
||||
let llm = spawn_capturing_llm(vec![shell_call("tc1"), openai_text("done")]).await;
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init(
|
||||
|
||||
@@ -1425,6 +1425,10 @@ async fn cancel_kills_inflight_tool_via_mcp_notification() {
|
||||
])
|
||||
.await;
|
||||
|
||||
// The agent stops buzz-dev-mcp with SIGKILL, so the server never deletes
|
||||
// its own temp dirs. Point its TMPDIR at a dir this test owns and removes.
|
||||
// Declared before `h` so it drops after the harness.
|
||||
let dev_mcp_tmp = tempfile::TempDir::new().expect("create buzz-dev-mcp TMPDIR");
|
||||
let mut h = Harness::spawn(&llm.url).await;
|
||||
let sid = init_session(
|
||||
&mut h,
|
||||
@@ -1432,7 +1436,7 @@ async fn cancel_kills_inflight_tool_via_mcp_notification() {
|
||||
"name": "dev",
|
||||
"command": &dev_mcp_bin,
|
||||
"args": [],
|
||||
"env": []
|
||||
"env": [{"name": "TMPDIR", "value": dev_mcp_tmp.path()}]
|
||||
}]),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { test } from "node:test";
|
||||
import { after, test } from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { spawn } from "node:child_process";
|
||||
|
||||
@@ -12,7 +12,11 @@ const desktopRoot = path.resolve(
|
||||
);
|
||||
const configDir = path.join(desktopRoot, "src-tauri");
|
||||
const wrapper = path.join(desktopRoot, "scripts/tauri-command.mjs");
|
||||
const fakeCli = path.join(tmpdir(), `buzz-fake-tauri-${process.pid}.mjs`);
|
||||
// Everything these tests write lives under one private root, removed once the
|
||||
// file's tests finish.
|
||||
const tempRoot = mkdtempSync(path.join(tmpdir(), "buzz-tauri-command-test-"));
|
||||
after(() => rmSync(tempRoot, { recursive: true, force: true }));
|
||||
const fakeCli = path.join(tempRoot, "fake-tauri.mjs");
|
||||
|
||||
writeFileSync(
|
||||
fakeCli,
|
||||
@@ -65,10 +69,8 @@ function packageVariant(variant, result, runnerArguments = []) {
|
||||
}
|
||||
|
||||
test("opposite Tauri package variants own private frontend artifacts", async () => {
|
||||
const resultRoot = path.join(tmpdir(), `buzz-tauri-results-${process.pid}`);
|
||||
mkdirSync(resultRoot, { recursive: true });
|
||||
const ossResult = path.join(resultRoot, "oss.json");
|
||||
const internalResult = path.join(resultRoot, "internal.json");
|
||||
const ossResult = path.join(tempRoot, "oss.json");
|
||||
const internalResult = path.join(tempRoot, "internal.json");
|
||||
|
||||
await Promise.all([
|
||||
packageVariant("0", ossResult),
|
||||
@@ -83,10 +85,7 @@ test("opposite Tauri package variants own private frontend artifacts", async ()
|
||||
});
|
||||
|
||||
test("private config precedes Cargo runner arguments", async () => {
|
||||
const result = path.join(
|
||||
tmpdir(),
|
||||
`buzz-tauri-runner-arguments-${process.pid}.json`,
|
||||
);
|
||||
const result = path.join(tempRoot, "runner-arguments.json");
|
||||
await packageVariant("0", result, [
|
||||
"--config",
|
||||
'{"bundle":{"active":false}}',
|
||||
@@ -106,10 +105,7 @@ test("private config precedes Cargo runner arguments", async () => {
|
||||
});
|
||||
|
||||
test("private frontendDist is never mistaken for a URL", async () => {
|
||||
const result = path.join(
|
||||
tmpdir(),
|
||||
`buzz-tauri-frontend-dist-${process.pid}.json`,
|
||||
);
|
||||
const result = path.join(tempRoot, "frontend-dist.json");
|
||||
await packageVariant("0", result);
|
||||
const invocation = JSON.parse(readFileSync(result, "utf8"));
|
||||
|
||||
|
||||
@@ -19,8 +19,12 @@ import 'package:video_player/video_player.dart';
|
||||
class _TempDirectory extends Fake
|
||||
with MockPlatformInterfaceMixin
|
||||
implements PathProviderPlatform {
|
||||
_TempDirectory(this.path);
|
||||
|
||||
final String path;
|
||||
|
||||
@override
|
||||
Future<String?> getTemporaryPath() async => Directory.systemTemp.path;
|
||||
Future<String?> getTemporaryPath() async => path;
|
||||
}
|
||||
|
||||
class _Player extends VideoPlayerPlatform {
|
||||
@@ -136,8 +140,14 @@ Future<_Player> _pumpVideo(
|
||||
final oldPlayer = VideoPlayerPlatform.instance;
|
||||
final oldPath = PathProviderPlatform.instance;
|
||||
final player = _Player(size: videoSize);
|
||||
// Registered first so it runs after the viewer is disposed. The viewer's
|
||||
// async delete of its buzz-video-* file never completes under FakeAsync.
|
||||
final temporaryDirectory = Directory.systemTemp.createTempSync(
|
||||
'video_controls_test_',
|
||||
);
|
||||
addTearDown(() => temporaryDirectory.deleteSync(recursive: true));
|
||||
VideoPlayerPlatform.instance = player;
|
||||
PathProviderPlatform.instance = _TempDirectory();
|
||||
PathProviderPlatform.instance = _TempDirectory(temporaryDirectory.path);
|
||||
addTearDown(() {
|
||||
VideoPlayerPlatform.instance = oldPlayer;
|
||||
PathProviderPlatform.instance = oldPath;
|
||||
|
||||
@@ -57,9 +57,12 @@ import '../../../helpers/widget_helpers.dart';
|
||||
class _FakePathProviderPlatform extends Fake
|
||||
with MockPlatformInterfaceMixin
|
||||
implements PathProviderPlatform {
|
||||
_FakePathProviderPlatform(this.temporaryPath);
|
||||
|
||||
final String temporaryPath;
|
||||
|
||||
@override
|
||||
Future<String?> getTemporaryPath() async =>
|
||||
Directory.systemTemp.resolveSymbolicLinksSync();
|
||||
Future<String?> getTemporaryPath() async => temporaryPath;
|
||||
}
|
||||
|
||||
/// Fake VideoPlayerPlatform that tracks `dispose` calls and optionally
|
||||
@@ -370,7 +373,16 @@ void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
setUp(() {
|
||||
PathProviderPlatform.instance = _FakePathProviderPlatform();
|
||||
// The viewer's async delete of its downloaded buzz-video-* file never
|
||||
// completes under FakeAsync, so give each test its own temp dir and
|
||||
// remove it synchronously.
|
||||
final temporaryDirectory = Directory(
|
||||
Directory.systemTemp.resolveSymbolicLinksSync(),
|
||||
).createTempSync('video_viewer_test_');
|
||||
addTearDown(() => temporaryDirectory.deleteSync(recursive: true));
|
||||
PathProviderPlatform.instance = _FakePathProviderPlatform(
|
||||
temporaryDirectory.path,
|
||||
);
|
||||
});
|
||||
|
||||
// Transport: request sink must be closed before send().
|
||||
|
||||
@@ -1417,7 +1417,7 @@ void main() {
|
||||
pickGalleryImage: () async => null,
|
||||
transcodeVideoToMp4: (path) async {
|
||||
transcodeCalled = true;
|
||||
final outDir = await Directory.systemTemp.createTemp('transcode_');
|
||||
final outDir = await tempFile.parent.createTemp('transcode_');
|
||||
final outFile = File('${outDir.path}/out.mp4');
|
||||
await outFile.writeAsBytes(buildFtypHeader('isom'));
|
||||
return outFile.path;
|
||||
@@ -1466,7 +1466,7 @@ void main() {
|
||||
transcodeVideoToMp4: (path) async {
|
||||
transcodeCalled = true;
|
||||
// Mock transcoding: write an "MP4" file
|
||||
final outDir = await Directory.systemTemp.createTemp('transcode_');
|
||||
final outDir = await tempFile.parent.createTemp('transcode_');
|
||||
final outFile = File('${outDir.path}/out.mp4');
|
||||
await outFile.writeAsBytes(buildFtypHeader('isom'));
|
||||
return outFile.path;
|
||||
@@ -1576,9 +1576,7 @@ void main() {
|
||||
pickGalleryVideo: () async => xfile,
|
||||
pickGalleryImage: () async => null,
|
||||
transcodeVideoToMp4: (path) async {
|
||||
final outDir = await Directory.systemTemp.createTemp(
|
||||
'transcode_',
|
||||
);
|
||||
final outDir = await tempFile.parent.createTemp('transcode_');
|
||||
final outFile = File('${outDir.path}/out.mp4');
|
||||
await outFile.writeAsBytes(buildFtypHeader('isom'));
|
||||
return outFile.path;
|
||||
@@ -1635,9 +1633,7 @@ void main() {
|
||||
pickGalleryVideo: () async => xfile,
|
||||
pickGalleryImage: () async => null,
|
||||
transcodeVideoToMp4: (path) async {
|
||||
final outDir = await Directory.systemTemp.createTemp(
|
||||
'transcode_',
|
||||
);
|
||||
final outDir = await tempFile.parent.createTemp('transcode_');
|
||||
final outFile = File('${outDir.path}/out.mp4');
|
||||
await outFile.writeAsBytes(buildFtypHeader('isom'));
|
||||
return outFile.path;
|
||||
|
||||
Reference in New Issue
Block a user