test: remove the scratch directories the fixtures leave behind

`env::temp_dir().join(format!("tty7-x-{pid}"))`, wiped on the way in and
left behind on the way out, reads as self-cleaning and is not. The pid is
in the name so two concurrent `cargo test` runs cannot share a fixture —
which also means a run never finds the previous run's directory to wipe.
One directory per fixture per run, kept forever: this working copy had
25,639 of them, and `cargo test` added about 37 more each time.

Two modules account for 10,904 of those, and both build every fixture
through one helper, so both are one edit: `ssh_config` (18 fixtures) and
`cli_install` (19). They now go through `testutil::temp_root`, which
hands back a guard that removes the tree when it drops — the pattern
`core::git::log` already uses, rather than a fourth copy of it.

Measured rather than assumed: a full bin-test run leaves both prefixes at
exactly the count they started at (5217 and 5687), where they used to
grow every run. The remaining 29 per run are the fixtures in modules not
converted here.

The guard is deliberately not `Clone` — copying something that deletes a
directory on drop is not a copy — so three sites that wanted a plain path
say `to_path_buf()`. Eight more chained `tmpdir(..).join(..)`, where the
guard is a temporary that drops at the end of the statement and takes the
directory with it before the test looks at it; the test suite caught that
as a failure and they now bind the guard first.

`pin_test_config_dir` keeps its uncleaned directory on purpose: it sets a
process-global config dir, so no scope owns it. `view.rs` was spelling
that function out by hand and now calls it.
This commit is contained in:
l0ng-ai
2026-08-16 01:31:32 +08:00
parent 99994553ef
commit cde2068ee6
5 changed files with 131 additions and 28 deletions
+37 -17
View File
@@ -730,12 +730,8 @@ mod unix_tests {
std::fs::write(p, b"#!/bin/sh\n").unwrap();
}
fn tmpdir(tag: &str) -> PathBuf {
let dir =
std::env::temp_dir().join(format!("tty7-cli-install-{tag}-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
dir
fn tmpdir(tag: &str) -> crate::testutil::TempRoot {
crate::testutil::temp_root(&format!("cli-install-{tag}"))
}
#[test]
@@ -754,7 +750,10 @@ mod unix_tests {
#[test]
fn an_unrelated_binary_named_tty7_is_left_alone() {
let dir = tmpdir("occupied");
let bin = tmpdir("occupied-src").join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before bin was used.
let occupied_src_dir = tmpdir("occupied-src");
let bin = occupied_src_dir.join("tty7");
touch(&bin);
// Someone's own build, installed by hand.
touch(&dir.join("tty7"));
@@ -769,8 +768,14 @@ mod unix_tests {
#[test]
fn our_own_link_is_recognised_and_then_repointed_on_upgrade() {
let dir = tmpdir("relink");
let v1 = tmpdir("relink-v1").join("tty7");
let v2 = tmpdir("relink-v2").join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before v1 was used.
let relink_v1_dir = tmpdir("relink-v1");
let v1 = relink_v1_dir.join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before v2 was used.
let relink_v2_dir = tmpdir("relink-v2");
let v2 = relink_v2_dir.join("tty7");
touch(&v1);
touch(&v2);
@@ -789,9 +794,15 @@ mod unix_tests {
#[test]
fn a_link_aimed_somewhere_deliberate_is_not_hijacked() {
let dir = tmpdir("deliberate");
let bin = tmpdir("deliberate-src").join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before bin was used.
let deliberate_src_dir = tmpdir("deliberate-src");
let bin = deliberate_src_dir.join("tty7");
touch(&bin);
let elsewhere = tmpdir("deliberate-other").join("my-terminal");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before elsewhere was used.
let deliberate_other_dir = tmpdir("deliberate-other");
let elsewhere = deliberate_other_dir.join("my-terminal");
touch(&elsewhere);
std::os::unix::fs::symlink(&elsewhere, dir.join("tty7")).unwrap();
@@ -804,8 +815,14 @@ mod unix_tests {
#[test]
fn a_copy_we_made_stays_ours_after_the_user_moves_off_the_appimage() {
let dir = tmpdir("appimage-migrate");
let v1 = tmpdir("appimage-mount-1").join("tty7");
let v2 = tmpdir("appimage-mount-2").join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before v1 was used.
let appimage_mount_1_dir = tmpdir("appimage-mount-1");
let v1 = appimage_mount_1_dir.join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before v2 was used.
let appimage_mount_2_dir = tmpdir("appimage-mount-2");
let v2 = appimage_mount_2_dir.join("tty7");
touch(&v1);
std::fs::write(&v2, b"#!/bin/sh\n# a later build\n").unwrap();
@@ -846,7 +863,10 @@ mod unix_tests {
fn an_occupied_directory_does_not_end_the_search() {
let taken = tmpdir("scan-taken");
let free = tmpdir("scan-free");
let bin = tmpdir("scan-src").join("tty7");
// Bound rather than chained: the guard removes the directory when it
// drops, and a temporary would drop before bin was used.
let scan_src_dir = tmpdir("scan-src");
let bin = scan_src_dir.join("tty7");
touch(&bin);
touch(&taken.join("tty7"));
@@ -869,11 +889,11 @@ mod unix_tests {
touch(&early.join("tty7"));
touch(&ours.join("tty7"));
let path = vec![early.clone(), ours.clone()];
let path = vec![early.to_path_buf(), ours.to_path_buf()];
assert_eq!(first_cli_on(&path), Some(early.join("tty7")));
// Our own directory first: no shadow.
assert_eq!(
first_cli_on(&[ours.clone(), early.clone()]),
first_cli_on(&[ours.to_path_buf(), early.to_path_buf()]),
Some(ours.join("tty7"))
);
@@ -881,7 +901,7 @@ mod unix_tests {
let dangling = tmpdir("shadow-dangling");
std::os::unix::fs::symlink(dangling.join("gone"), dangling.join("tty7")).unwrap();
assert_eq!(
first_cli_on(&[dangling, ours.clone()]),
first_cli_on(&[dangling.to_path_buf(), ours.to_path_buf()]),
Some(ours.join("tty7"))
);
}
+2 -8
View File
@@ -1443,13 +1443,7 @@ mod tests {
f.set_modified(ahead).unwrap();
}
fn temp_root(name: &str) -> PathBuf {
let dir = std::env::temp_dir().join(format!(
"tty7-ssh-config-test-{name}-{}",
std::process::id()
));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
dir
fn temp_root(name: &str) -> crate::testutil::TempRoot {
crate::testutil::temp_root(&format!("ssh-config-test-{name}"))
}
}
+2
View File
@@ -6,6 +6,8 @@
mod core;
mod daemon;
mod terminal;
#[cfg(test)]
mod testutil;
mod ui;
use crate::core::config::Config;
+1 -3
View File
@@ -9802,9 +9802,7 @@ mod gpui_tests {
#[gpui::test]
fn insert_newline_action_extends_the_line_and_enter_submits_it(cx: &mut TestAppContext) {
let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id()));
std::fs::create_dir_all(&dir).ok();
crate::core::config::set_config_dir(dir);
crate::core::config::pin_test_config_dir();
let (window, mut daemon) = harness(cx);
prompt_ready(&window, cx, &mut daemon);
+89
View File
@@ -0,0 +1,89 @@
//! Scratch directories for tests, removed when the test ends.
//!
//! Every fixture here used to be `env::temp_dir().join(format!("tty7-x-{pid}"))`,
//! wiped on the way *in* and left behind on the way out. That reads as
//! self-cleaning and is not: the pid is in the name so two concurrent
//! `cargo test` runs cannot share a fixture, which also means a run never
//! finds the previous run's directory to wipe. One directory per fixture per
//! run, kept forever — this working copy had 25,639 of them.
//!
//! Dropping the guard is what removes the tree, so a test that panics still
//! cleans up: unwinding runs destructors, and the assertion failure is the
//! output that matters rather than the litter left beside it.
use std::path::{Path, PathBuf};
/// A directory under the system temp dir that removes itself when dropped.
///
/// Derefs to [`Path`], so it stands in for the `PathBuf` these fixtures used
/// to hand back: `root.join("x")` and `&root` both keep working.
///
/// Bind it to a name — `let root = temp_root("x")` — rather than calling a
/// method on it directly. A temporary would be dropped at the end of the
/// statement that made it, taking the directory with it before the test
/// looked at it.
pub struct TempRoot(PathBuf);
impl Drop for TempRoot {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
impl std::ops::Deref for TempRoot {
type Target = Path;
fn deref(&self) -> &Path {
&self.0
}
}
impl AsRef<Path> for TempRoot {
fn as_ref(&self) -> &Path {
&self.0
}
}
/// A fresh, empty `tty7-<name>-<pid>` directory that lasts as long as the
/// returned guard.
///
/// The pid stays in the name: it is what keeps two concurrent `cargo test`
/// runs off each other's fixture. It is the missing *removal*, not the
/// unique name, that made these accumulate.
pub fn temp_root(name: &str) -> TempRoot {
let dir = std::env::temp_dir().join(format!("tty7-{name}-{}", std::process::id()));
// Still wiped on the way in: a previous run that was killed hard enough
// to skip its destructors must not leave a fixture behind that this one
// would then read as its own.
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
TempRoot(dir)
}
#[cfg(test)]
mod tests {
use super::temp_root;
#[test]
fn a_scratch_directory_is_gone_once_its_guard_is() {
let path = {
let root = temp_root("testutil-selfcheck");
let path = root.to_path_buf();
assert!(path.is_dir(), "the fixture exists while the guard is held");
std::fs::write(root.join("f"), b"x").unwrap();
path
};
assert!(!path.exists(), "and is removed with the guard: {path:?}");
}
#[test]
fn a_second_call_starts_from_an_empty_directory() {
let first = temp_root("testutil-reuse");
std::fs::write(first.join("stale"), b"x").unwrap();
let path = first.to_path_buf();
std::mem::forget(first); // simulate a run that never dropped its guard
let again = temp_root("testutil-reuse");
assert_eq!(*again, *path, "same name, same pid, same directory");
assert!(!again.join("stale").exists(), "wiped on the way in");
}
}