Move all the scratch file generation in to a class, and in to a specific subdir

ofborg/.gitignore

@@ -1,6 +1,4 @@
 target
 rust-amqp
 test-scratch
-scratch-*
 *.bk
-test-message-log-scratch/

ofborg/src/lib.rs

@@ -39,6 +39,7 @@ pub mod tagger;
 pub mod asynccmd;
 pub mod notifyworker;
 pub mod writetoline;
+pub mod test_scratch;
 
 pub mod ofborg {
     pub use asynccmd;
@@ -60,6 +61,7 @@ pub mod ofborg {
     pub use outpathdiff;
     pub use tagger;
     pub use writetoline;
+    pub use test_scratch;
 
 
 }

ofborg/src/tasks/build.rs

@@ -315,6 +315,7 @@ mod tests {
     use notifyworker::SimpleNotifyWorker;
     use std::process::{Command, Stdio};
     use std::vec::IntoIter;
+    use ofborg::test_scratch::TestScratch;
 
     fn nix() -> nix::Nix {
         nix::Nix::new("x86_64-linux".to_owned(), "daemon".to_owned(), 1800)
@@ -324,22 +325,8 @@ mod tests {
         return Path::new(env!("CARGO_MANIFEST_DIR")).join(component);
     }
 
-    fn scratch_dir() -> PathBuf {
-        tpath("./test-scratch")
-    }
-
-    fn cleanup_scratch() {
-        Command::new("rm")
-            .arg("-rf")
-            .arg(&scratch_dir())
-            .status()
-            .expect("cleanup of test-scratch should work");
-    }
-
-    fn make_worker() -> BuildWorker {
-        cleanup_scratch();
-
-        let cloner = checkout::cached_cloner(&scratch_dir());
+    fn make_worker(path: &Path) -> BuildWorker {
+        let cloner = checkout::cached_cloner(path);
         let nix = nix();
         let worker = BuildWorker::new(
             cloner,
@@ -352,9 +339,11 @@ mod tests {
         return worker;
     }
 
-    fn make_pr_repo() -> String {
+    fn make_pr_repo(bare: &Path, co: &Path) -> String {
         let output = Command::new("./make-pr.sh")
             .current_dir(tpath("./test-srcs"))
+            .arg(bare)
+            .arg(co)
             .stderr(Stdio::null())
             .stdout(Stdio::piped())
             .output()
@@ -390,17 +379,23 @@ mod tests {
 
     #[test]
     pub fn test_simple_build() {
-        let worker = make_worker();
+        let p = TestScratch::new_dir("build-simple-build-working");
+        let bare_repo = TestScratch::new_dir("build-simple-build-bare");
+        let co_repo = TestScratch::new_dir("build-simple-build-co");
+
+        let head_sha = make_pr_repo(&bare_repo.path(), &co_repo.path());
+        let worker = make_worker(&p.path());
+
 
         let job = buildjob::BuildJob {
             attrs: vec!["success".to_owned()],
             pr: Pr {
-                head_sha: make_pr_repo(),
+                head_sha: head_sha,
                 number: 1,
                 target_branch: Some("master".to_owned()),
             },
             repo: Repo {
-                clone_url: tpath("./test-srcs/bare-repo").to_str().unwrap().to_owned(),
+                clone_url: bare_repo.path().to_str().unwrap().to_owned(),
                 full_name: "test-git".to_owned(),
                 name: "nixos".to_owned(),
                 owner: "ofborg-test".to_owned(),

ofborg/src/tasks/log_message_collector.rs

@@ -163,31 +163,13 @@ impl worker::SimpleWorker for LogMessageCollector {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::process::Command;
     use std::io::Read;
-    use std::path::Path;
+    use std::path::PathBuf;
     use ofborg::worker::SimpleWorker;
+    use ofborg::test_scratch::TestScratch;
 
-    fn tpath(component: &str) -> PathBuf {
-        return Path::new(env!("CARGO_MANIFEST_DIR")).join(component);
-    }
-
-    fn scratch_dir(name: &str) -> PathBuf {
-        tpath(&format!("./test-message-log-scratch-{}", name))
-    }
-
-    fn cleanup_scratch(name: &str) {
-        Command::new("rm")
-            .arg("-rf")
-            .arg(&scratch_dir(name))
-            .status()
-            .expect("cleanup of scratch_dir should work");
-    }
-
-    fn make_worker(name: &str) -> LogMessageCollector {
-        cleanup_scratch(name);
-
-        LogMessageCollector::new(scratch_dir(name), 3)
+    fn make_worker(path: PathBuf) -> LogMessageCollector {
+        LogMessageCollector::new(path, 3)
     }
 
     fn make_from(id: &str) -> LogFrom {
@@ -199,12 +181,14 @@ mod tests {
 
     #[test]
     fn test_handle_for() {
+        let p = TestScratch::new_dir("log-message-collector-handle_for");
+
         let a = make_from("a.foo/123");
         let b = make_from("b.foo/123");
         let c = make_from("c.foo/123");
         let d = make_from("d.foo/123");
 
-        let mut worker = make_worker("handle_for");
+        let mut worker = make_worker(p.path());
         assert!(worker.handle_for(&a).is_ok());
         assert!(worker.handle_for(&b).is_ok());
         assert!(worker.handle_for(&c).is_ok());
@@ -214,7 +198,8 @@ mod tests {
 
     #[test]
     fn test_path_for() {
-        let worker = make_worker("path_for");
+        let p = TestScratch::new_dir("log-message-collector-path_for");
+        let worker = make_worker(p.path());
 
         let path = worker
             .path_for(&LogFrom {
@@ -224,13 +209,14 @@ mod tests {
             .expect("the path should be valid");
 
 
-        assert!(path.starts_with(scratch_dir("path_for")));
+        assert!(path.starts_with(p.path()));
         assert!(path.ends_with("my-routing-key/my-attempt-id"));
     }
 
     #[test]
     fn test_path_for_malicious() {
-        let worker = make_worker("for_malicious");
+        let p = TestScratch::new_dir("log-message-collector-for_malicious");
+        let worker = make_worker(p.path());
 
         let path = worker.path_for(&LogFrom {
             attempt_id: String::from("./../../"),
@@ -260,7 +246,9 @@ mod tests {
 
     #[test]
     fn test_open_log() {
-        let worker = make_worker("open-log");
+        let p = TestScratch::new_dir("log-message-collector-open_log");
+        let worker = make_worker(p.path());
+
         assert!(
             worker
                 .open_log(worker.path_for(&make_from("a")).unwrap())
@@ -286,8 +274,10 @@ mod tests {
             },
         };
 
+        let p = TestScratch::new_dir("log-message-collector-path_for");
+
         {
-            let mut worker = make_worker("simple-build");
+            let mut worker = make_worker(p.path());
             assert_eq!(vec![worker::Action::Ack], worker.consumer(&job));
 
             job.message.line_number = 5;
@@ -301,19 +291,17 @@ mod tests {
             assert_eq!(vec![worker::Action::Ack], worker.consumer(&job));
         }
 
-        let root = scratch_dir("simple-build");
-
-        let mut p = root.clone();
+        let mut pr = p.path();
         let mut s = String::new();
-        p.push("routing-key-foo/attempt-id-foo");
-        File::open(p).unwrap().read_to_string(&mut s).unwrap();
+        pr.push("routing-key-foo/attempt-id-foo");
+        File::open(pr).unwrap().read_to_string(&mut s).unwrap();
         assert_eq!(&s, "line-1\n\n\n\nline-5\n");
 
 
-        let mut p = root.clone();
+        let mut pr = p.path();
         let mut s = String::new();
-        p.push("routing-key-foo/my-other-attempt");
-        File::open(p).unwrap().read_to_string(&mut s).unwrap();
+        pr.push("routing-key-foo/my-other-attempt");
+        File::open(pr).unwrap().read_to_string(&mut s).unwrap();
         assert_eq!(&s, "\n\nline-3\n");
     }
 }

ofborg/src/test_scratch.rs

@@ -0,0 +1,48 @@
+use std::fs;
+use std::path::{Path,PathBuf};
+use std::process::Command;
+
+pub struct TestScratch {
+    root: PathBuf
+}
+
+impl TestScratch {
+    pub fn new_dir(ident: &str) -> TestScratch {
+        let path = TestScratch {
+            root: Path::new(env!("CARGO_MANIFEST_DIR"))
+                .join("test-scratch")
+                .join("dirs")
+                .join(&format!("dir-{}", ident)),
+        };
+        fs::create_dir_all(path.root.parent().unwrap()).unwrap();
+
+        return path;
+    }
+
+    pub fn new_file(ident: &str) -> TestScratch {
+        let path = TestScratch {
+            root: Path::new(env!("CARGO_MANIFEST_DIR"))
+                .join("test-scratch")
+                .join("files")
+                .join(&format!("file-{}", ident)),
+        };
+        fs::create_dir_all(path.root.parent().unwrap()).unwrap();
+
+        return path;
+    }
+
+    pub fn path(&self) -> PathBuf {
+        self.root.clone()
+    }
+
+}
+
+impl Drop for TestScratch {
+    fn drop(&mut self) {
+        Command::new("rm")
+            .arg("-rf")
+            .arg(self.root.clone())
+            .status()
+            .expect("cleanup of test-scratch should work");
+    }
+}

ofborg/src/writetoline.rs

@@ -37,37 +37,19 @@ pub fn write_to_line(rw: &mut File, line: usize, data: &str) {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::process::Command;
     use std::path::Path;
-    use std::path::PathBuf;
     use std::fs::File;
     use std::io::Read;
     use std::fs::OpenOptions;
+    use ofborg::test_scratch::TestScratch;
 
-    fn tpath(component: &str) -> PathBuf {
-        return Path::new(env!("CARGO_MANIFEST_DIR")).join(component);
-    }
-
-    fn scratch_file(name: &str) -> PathBuf {
-        tpath(&format!("./scratch-write-to-line-{}", name))
-    }
-
-    fn cleanup_scratch(name: &str) {
-        Command::new("rm")
-            .arg("-f")
-            .arg(&scratch_file(name))
-            .status()
-            .expect("cleanup of scratch_dir should work");
-    }
-
-    fn testfile(name: &str) -> File {
-        cleanup_scratch(&name);
+    fn testfile(path: &Path) -> File {
         OpenOptions::new()
             .read(true)
             .write(true)
             .truncate(true)
             .create(true)
-            .open(scratch_file(&name))
+            .open(path)
             .expect("failed to open scratch file")
     }
 
@@ -83,7 +65,8 @@ mod tests {
 
     #[test]
     fn test_writer_line_ordered() {
-        let mut f = testfile("ordered");
+        let p = TestScratch::new_file("writetoline-ordered");
+        let mut f = testfile(&p.path());
 
         assert_file_content(&mut f, "");
         write_to_line(&mut f, 0, "hello");
@@ -96,7 +79,8 @@ mod tests {
 
     #[test]
     fn test_writer_line_unordered() {
-        let mut f = testfile("unordered");
+        let p = TestScratch::new_file("writetoline-unordered");
+        let mut f = testfile(&p.path());
 
         assert_file_content(&mut f, "");
         write_to_line(&mut f, 2, ":)");
@@ -112,7 +96,8 @@ mod tests {
 
     #[test]
     fn test_writer_line_unordered_long() {
-        let mut f = testfile("unordered-long");
+        let p = TestScratch::new_file("writetoline-unordered-long");
+        let mut f = testfile(&p.path());
 
         assert_file_content(&mut f, "");
         write_to_line(
@@ -149,7 +134,8 @@ mod tests {
 
     #[test]
     fn test_writer_line_unordered_longish() {
-        let mut f = testfile("unordered-longish");
+        let p = TestScratch::new_file("writetoline-unordered-longish");
+        let mut f = testfile(&p.path());
 
         assert_file_content(&mut f, "");
         write_to_line(&mut f, 2, "hello");
@@ -164,7 +150,8 @@ mod tests {
 
     #[test]
     fn test_writer_line_middle() {
-        let mut f = testfile("middle");
+        let p = TestScratch::new_file("writetoline-middle");
+        let mut f = testfile(&p.path());
 
         assert_file_content(&mut f, "");
         write_to_line(&mut f, 5, "hello");

ofborg/test-srcs/.gitignore

@@ -1,2 +0,0 @@
-bare-repo
-repo-co

ofborg/test-srcs/make-pr.sh

@@ -1,26 +1,27 @@
 #!/bin/sh
 
 set -eux
+set -o pipefail
 
+bare=$1
+co=$2
 
 (
-    rm -rf bare-repo repo-co
-
-    git init --bare bare-repo
-    git clone ./bare-repo ./repo-co
+    git init --bare "$bare"
+    git clone "$bare" "$co"
 
 
     (
-        cp build/* repo-co/
-        cd repo-co
+        cp build/* "$co/"
+        cd  "$co/"
         git add .
         git commit --no-gpg-sign -m "initial repo commit"
         git push origin master
     )
 
     (
-        cp build-pr/* repo-co/
-        cd repo-co
+        cp build-pr/*  "$co/"
+        cd "$co/"
         git checkout -b my-cool-pr
         git add .
         git commit --no-gpg-sign -m "check out this cool PR"
@@ -30,6 +31,6 @@ set -eux
 ) >&2
 
 (
-    cd repo-co
+    cd  "$co/"
     git rev-parse HEAD
 )