From a78f1bc58a32bf3ca5834997f419d2a764748373 Mon Sep 17 00:00:00 2001 From: Luca P Date: Sat, 26 Mar 2022 21:41:34 +0000 Subject: [PATCH 1/7] Change test expectations to match the new behaviour we will be implementing. --- tests/integration_tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index bc5b1de..b2afafb 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -33,7 +33,7 @@ fn private_2() { #[assay(include = ["Cargo.toml", "src/lib.rs"])] fn include() { - assert!(fs::metadata("src/lib.rs")?.is_file()); + assert!(fs::metadata("lib.rs")?.is_file()); assert!(fs::metadata("Cargo.toml")?.is_file()); } @@ -110,7 +110,7 @@ async fn one_test_to_call_it_all() { assert_eq!(env::var("BADDOGS")?, "false"); assert_eq!(fs::read_to_string("setup")?, "Value: 5"); assert!(PathBuf::from("Cargo.toml").exists()); - assert!(PathBuf::from("src/lib.rs").exists()); + assert!(PathBuf::from("lib.rs").exists()); // Removing this actually causes the test to fail panic!(); @@ -133,7 +133,7 @@ async fn one_test_to_call_it_all_2() { assert_eq!(env::var("BADDOGS")?, "false"); assert_eq!(fs::read_to_string("setup")?, "Value: 5"); assert!(PathBuf::from("Cargo.toml").exists()); - assert!(PathBuf::from("src/lib.rs").exists()); + assert!(PathBuf::from("lib.rs").exists()); // Removing this actually causes the test to fail panic!(); From 181e4bfd510aaab3bb00eff037d27a3eb8f5a7f1 Mon Sep 17 00:00:00 2001 From: Luca P Date: Sat, 26 Mar 2022 21:53:51 +0000 Subject: [PATCH 2/7] Happy tests. --- src/lib.rs | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 27b5225..aabdd5f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,8 +25,8 @@ pub use rusty_fork::{fork, rusty_fork_id, rusty_fork_test_name, ChildWrapper}; use std::{ env, error::Error, - fs::{copy, create_dir_all}, - path::{Component, Path, PathBuf}, + fs::{copy}, + path::{Path, PathBuf}, }; use tempfile::{Builder, TempDir}; @@ -61,25 +61,11 @@ impl PrivateFS { // Get our working directory let dir = self.directory.path().to_owned(); - // Make the relative path of the file in relation to our temp file - // system based on if it was absolute or not - let relative = if !is_relative { - inner_path - .components() - .filter(|c| *c != Component::RootDir) - .collect::() - } else { - path.as_ref().into() - }; - - // If the relative path to the file includes parent directories create - // them - if let Some(parent) = relative.parent() { - create_dir_all(dir.join(parent))?; - } - // Copy the file over from the file system into the temp file system - copy(inner_path, dir.join(relative))?; + // We mount all included files in the root directory of the test's private filesystem + // TODO: return meaningful error message + let filename = inner_path.file_name().unwrap().to_owned(); + copy(inner_path, dir.join(filename))?; Ok(()) } From 5851b2a88a97e71f7d211855f0045ef16b2d6e9c Mon Sep 17 00:00:00 2001 From: Luca P Date: Sat, 26 Mar 2022 21:55:41 +0000 Subject: [PATCH 3/7] Add a test that tries to include a file using an explicit destination path. --- tests/integration_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index b2afafb..2e9ab34 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -31,10 +31,11 @@ fn private_2() { assert_eq!("This is a test\nprivate 2\n", &fs::read_to_string("test")?); } -#[assay(include = ["Cargo.toml", "src/lib.rs"])] +#[assay(include = ["Cargo.toml", "src/lib.rs", ("HOW_TO_USE.md", "docs/GUIDE.md")])] fn include() { assert!(fs::metadata("lib.rs")?.is_file()); assert!(fs::metadata("Cargo.toml")?.is_file()); + assert!(fs::metadata("docs/GUIDE.md")?.is_file()); } #[assay(should_panic)] From 27ddf5f035839e95cd7710c8caf5fee37eef3cc2 Mon Sep 17 00:00:00 2001 From: Luca P Date: Sat, 26 Mar 2022 22:24:39 +0000 Subject: [PATCH 4/7] Add support for an optional destination path. --- assay-proc-macro/src/lib.rs | 32 ++++++++++++++++++++++++++++---- src/lib.rs | 35 ++++++++++++++++++++++++++++------- tests/integration_tests.rs | 4 ++-- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/assay-proc-macro/src/lib.rs b/assay-proc-macro/src/lib.rs index 917fed8..703ee50 100644 --- a/assay-proc-macro/src/lib.rs +++ b/assay-proc-macro/src/lib.rs @@ -14,7 +14,7 @@ use syn::{ }; struct AssayAttribute { - include: Option>, + include: Option)>>, should_panic: bool, env: Option>, setup: Option, @@ -48,7 +48,27 @@ impl Parse for AssayAttribute { Expr::Lit(ExprLit { lit: Lit::Str(lit_str), .. - }) => Some(lit_str.value()), + }) => { + let value = lit_str.value(); + Some((value, None)) + }, + Expr::Tuple(tuple) => { + let mut elements = Vec::new(); + for e in tuple.elems.into_iter() { + if let Expr::Lit(ExprLit { lit: Lit::Str(lit_str), ..}) = e { + elements.push(lit_str.value()); + } else { + // TODO: Should we return a parsing error to the user here? How? + return None; + } + } + if elements.len() == 2 { + Some((elements[0].clone(), Some(elements[1].clone()))) + } else { + // TODO: Should we return a parsing error to the user here? How? + None + } + } _ => None, }) .collect(), @@ -112,10 +132,14 @@ pub fn assay(attr: TokenStream, item: TokenStream) -> TokenStream { let mut out = quote! { let fs = assay::PrivateFS::new()?; }; - for file in include { + for (source_path, destination_path) in include { + let destination_fragment = match destination_path { + None => quote!(std::option::Option::None), + Some(p) => quote!(std::option::Option::Some(#p)) + }; out = quote! { #out - fs.include(#file)?; + fs.include(#source_path, #destination_fragment)?; }; } out diff --git a/src/lib.rs b/src/lib.rs index aabdd5f..05f2bee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,6 +28,7 @@ use std::{ fs::{copy}, path::{Path, PathBuf}, }; +use std::fs::create_dir_all; use tempfile::{Builder, TempDir}; #[doc(hidden)] @@ -47,25 +48,45 @@ impl PrivateFS { }) } - pub fn include(&self, path: impl AsRef) -> Result<(), Box> { + pub fn include(&self, source_path: impl AsRef, destination_path: Option<&str>) -> Result<(), Box> { // Get our pathbuf to the file to include - let mut inner_path = path.as_ref().to_owned(); + let mut inner_path = source_path.as_ref().to_owned(); // If the path given is not absolute then it's relative to the dir we // ran the test from let is_relative = inner_path.is_relative(); if is_relative { - inner_path = self.ran_from.join(&path); + inner_path = self.ran_from.join(&source_path); } // Get our working directory let dir = self.directory.path().to_owned(); + let destination_path = match destination_path { + None => { + // If the destination path is unspecified, we mount the file in the root directory + // of the test's private filesystem + // TODO: return meaningful error message + let filename = inner_path.file_name().unwrap().to_owned(); + dir.join(filename) + }, + Some(p) => { + let p = PathBuf::from(p); + if !p.is_relative() { + // TODO: do we want to panic here? + panic!("The destination path for included files must be a relative path. {:?} isn't.", p); + } + // If the relative path to the file includes parent directories create + // them + if let Some(parent) = p.parent() { + create_dir_all(dir.join(parent))?; + } + dir.join(p) + } + }; + // Copy the file over from the file system into the temp file system - // We mount all included files in the root directory of the test's private filesystem - // TODO: return meaningful error message - let filename = inner_path.file_name().unwrap().to_owned(); - copy(inner_path, dir.join(filename))?; + copy(inner_path, destination_path)?; Ok(()) } diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 2e9ab34..4afe682 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -96,7 +96,7 @@ fn setup_teardown_test_2() { #[assay( setup = setup_func_2(), - include = ["Cargo.toml", "src/lib.rs"], + include = ["Cargo.toml", ("src/lib.rs", "src/lib.rs")], env = [ ("GOODBOY", "Bukka"), ("BADDOGS", "false") @@ -111,7 +111,7 @@ async fn one_test_to_call_it_all() { assert_eq!(env::var("BADDOGS")?, "false"); assert_eq!(fs::read_to_string("setup")?, "Value: 5"); assert!(PathBuf::from("Cargo.toml").exists()); - assert!(PathBuf::from("lib.rs").exists()); + assert!(PathBuf::from("src/lib.rs").exists()); // Removing this actually causes the test to fail panic!(); From 4c1d3fa01653e8b377dba150ffc6f0ddab9e5616 Mon Sep 17 00:00:00 2001 From: Luca Palmieri Date: Fri, 1 Apr 2022 10:51:25 +0100 Subject: [PATCH 5/7] Make the second path parameter in `include` generic. Avoid an inference error by using the turbofish to force D=PathBuf when the destination path is unspecified. --- assay-proc-macro/src/lib.rs | 12 ++++++++---- src/lib.rs | 20 +++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/assay-proc-macro/src/lib.rs b/assay-proc-macro/src/lib.rs index 703ee50..ebd5362 100644 --- a/assay-proc-macro/src/lib.rs +++ b/assay-proc-macro/src/lib.rs @@ -51,11 +51,15 @@ impl Parse for AssayAttribute { }) => { let value = lit_str.value(); Some((value, None)) - }, + } Expr::Tuple(tuple) => { let mut elements = Vec::new(); for e in tuple.elems.into_iter() { - if let Expr::Lit(ExprLit { lit: Lit::Str(lit_str), ..}) = e { + if let Expr::Lit(ExprLit { + lit: Lit::Str(lit_str), + .. + }) = e + { elements.push(lit_str.value()); } else { // TODO: Should we return a parsing error to the user here? How? @@ -134,8 +138,8 @@ pub fn assay(attr: TokenStream, item: TokenStream) -> TokenStream { }; for (source_path, destination_path) in include { let destination_fragment = match destination_path { - None => quote!(std::option::Option::None), - Some(p) => quote!(std::option::Option::Some(#p)) + None => quote!(std::option::Option::<::std::path::PathBuf>::None), + Some(p) => quote!(std::option::Option::Some(#p)), }; out = quote! { #out diff --git a/src/lib.rs b/src/lib.rs index 05f2bee..5cfee7a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,13 +22,13 @@ pub use pretty_assertions_sorted::{assert_eq, assert_eq_sorted, assert_ne}; #[doc(hidden)] pub use rusty_fork::{fork, rusty_fork_id, rusty_fork_test_name, ChildWrapper}; +use std::fs::create_dir_all; use std::{ env, error::Error, - fs::{copy}, + fs::copy, path::{Path, PathBuf}, }; -use std::fs::create_dir_all; use tempfile::{Builder, TempDir}; #[doc(hidden)] @@ -48,7 +48,11 @@ impl PrivateFS { }) } - pub fn include(&self, source_path: impl AsRef, destination_path: Option<&str>) -> Result<(), Box> { + pub fn include(&self, source_path: S, destination_path: Option) -> Result<(), Box> + where + S: AsRef, + D: AsRef, + { // Get our pathbuf to the file to include let mut inner_path = source_path.as_ref().to_owned(); @@ -69,12 +73,14 @@ impl PrivateFS { // TODO: return meaningful error message let filename = inner_path.file_name().unwrap().to_owned(); dir.join(filename) - }, + } Some(p) => { - let p = PathBuf::from(p); + let p = p.as_ref(); if !p.is_relative() { - // TODO: do we want to panic here? - panic!("The destination path for included files must be a relative path. {:?} isn't.", p); + panic!( + "The destination path for included files must be a relative path. {:?} isn't.", + p + ); } // If the relative path to the file includes parent directories create // them From e142cf0dc9029a1bc2731f8de1a52a2f28bedef0 Mon Sep 17 00:00:00 2001 From: Luca Palmieri Date: Fri, 1 Apr 2022 10:59:50 +0100 Subject: [PATCH 6/7] Polish up panic messages. --- src/lib.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5cfee7a..67c172a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,11 @@ impl PrivateFS { }) } - pub fn include(&self, source_path: S, destination_path: Option) -> Result<(), Box> + pub fn include( + &self, + source_path: S, + destination_path: Option, + ) -> Result<(), Box> where S: AsRef, D: AsRef, @@ -63,6 +67,13 @@ impl PrivateFS { inner_path = self.ran_from.join(&source_path); } + if !inner_path.is_file() { + panic!( + "The source path passed to `#[include()]` must point to a file. {:?} is not a file.", + inner_path + ); + } + // Get our working directory let dir = self.directory.path().to_owned(); @@ -70,9 +81,15 @@ impl PrivateFS { None => { // If the destination path is unspecified, we mount the file in the root directory // of the test's private filesystem - // TODO: return meaningful error message - let filename = inner_path.file_name().unwrap().to_owned(); - dir.join(filename) + match inner_path.file_name() { + Some(filename) => dir.join(filename), + None => { + panic!( + "Failed to extract the filename from the source path, {:?}.", + inner_path + ) + } + } } Some(p) => { let p = p.as_ref(); From 200ead6d35b607196e3df8f45748fb48bf0addbd Mon Sep 17 00:00:00 2001 From: Luca Palmieri Date: Fri, 1 Apr 2022 11:00:15 +0100 Subject: [PATCH 7/7] Remove todos. --- assay-proc-macro/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/assay-proc-macro/src/lib.rs b/assay-proc-macro/src/lib.rs index ebd5362..ffcd80b 100644 --- a/assay-proc-macro/src/lib.rs +++ b/assay-proc-macro/src/lib.rs @@ -62,14 +62,12 @@ impl Parse for AssayAttribute { { elements.push(lit_str.value()); } else { - // TODO: Should we return a parsing error to the user here? How? return None; } } if elements.len() == 2 { Some((elements[0].clone(), Some(elements[1].clone()))) } else { - // TODO: Should we return a parsing error to the user here? How? None } }