web browser (Servo)
git clone https://git.lucas.co/cce-browser.git
fix: the tests' scratch directories are scoped to the process
`session.rs`'s `temp_session` used a fixed `/tmp/cce-browser-session-test`,
and `main.rs`'s startup-arg test a fixed `/tmp/cce-browser-argv-test`, where
the sibling helpers in `pages.rs` already scope theirs by pid.
/tmp is one namespace shared by every user of the machine — the point
`cce-compositor/WORKSPACE.md` makes about `cce_runtime_dir` — so a fixed name
belongs to whoever ran first and the sticky bit denies it to everyone else.
Nearer to hand, two checkouts running their suites at once shared one
directory.
`temp_session`'s directory is deliberately NOT removed wholesale, here or at
the end of a test: its three tests run in parallel threads of one process and
so share the one pid-scoped directory, and a `remove_dir_all` would take a
sibling's file out from under it. Each owns a distinct file name and clears
just that. The argv test is the sole user of its own directory, so that one
goes whole.
Found by sweeping every crate's test binaries for real paths, after the same
class of bug had cce-designer's suite rewriting the user's state.kdl.
Co-Authored-By: Claude Opus 5 <[email protected]>
src/main.rs | 8 ++++++--
src/session.rs | 17 ++++++++++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/main.rs b/src/main.rs
index 57cacd2..1bccc00 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -3092,7 +3092,10 @@ mod tests {
#[test]
fn startup_arg_resolves_an_existing_path_to_a_file_url() {
- let dir = std::env::temp_dir().join("cce-browser-argv-test");
+ // Scoped to this process, like every other scratch directory in the
+ // crate: /tmp is one namespace shared by every user of the machine.
+ let dir = std::env::temp_dir()
+ .join(format!("cce-browser-argv-test-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let page = dir.join("page.html");
std::fs::write(&page, "<html></html>").unwrap();
@@ -3106,7 +3109,8 @@ mod tests {
let bar = parse_url_input(page.to_str().unwrap(), SEARCH).unwrap();
assert_eq!(bar.scheme(), "https");
- std::fs::remove_file(&page).unwrap();
+ // Sole user of this directory, so it can go whole.
+ let _ = std::fs::remove_dir_all(&dir);
}
#[cfg(feature = "wpe")]
diff --git a/src/session.rs b/src/session.rs
index c41bf2f..acadcf7 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -76,8 +76,23 @@ impl Session {
mod tests {
use super::*;
+ /// A session file in a scratch directory of this PROCESS's own.
+ ///
+ /// The name was fixed — `/tmp/cce-browser-session-test` — where the
+ /// sibling helpers in `pages.rs` already scope theirs by pid. /tmp is one
+ /// namespace shared by every user of the machine, so a fixed name belongs
+ /// to whoever ran first and the sticky bit denies it to everyone else;
+ /// nearer to hand, two checkouts running their suites at once shared one
+ /// directory.
+ ///
+ /// The directory is deliberately NOT removed wholesale, here or at the
+ /// end of a test: the three tests run in parallel threads of one process
+ /// and so share this one pid-scoped directory, and a `remove_dir_all`
+ /// would take a sibling's file out from under it. Each test owns a
+ /// distinct file name and clears just that.
fn temp_session(name: &str) -> Session {
- let dir = std::env::temp_dir().join("cce-browser-session-test");
+ let dir = std::env::temp_dir()
+ .join(format!("cce-browser-session-test-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join(name);
let _ = std::fs::remove_file(&path);