From 1a6262cd5fa73495e227cefee513da29f406de2e Mon Sep 17 00:00:00 2001 From: mcharsley Date: Mon, 27 Mar 2017 11:11:11 +0100 Subject: [PATCH] Fixed windows compatability (#21) * Deleted vertical whitespace In an attempt to get around a mistake I made when merging from the master fork * Added a default implementation for has_side_effects And removed all the "return false" implementations (as specified by most of the Matchers). * Added support for -exec and -execdir * Fixed path_to_testing_commandline * Minor tweaks from code review * Added support for -perm * Fixed string constant * Fixed error caused by merging changes * Tweaks after code review * Fixed windows compatability. This required - changing a lot of test code to supply/expect backslashes rather than slashes when running on windows - tweaking the way testing-commandline reported its args (debug format escapes backslashes when printing strings) - changing the NewerMatcher to get metadata from fs::metadata() rather than from File's metadata method (the latter caused mysterious test failures, which I didn't bother tracking down becasue the former is more efficient anyway) - hiding more of perm.rs behind #[cfg(unix)] to stop "unused X" warnings * Revert "Fixed windows compatability. This required" This reverts commit 0e2c38523761a300a7a0f05a0d7788a13e7b1312. * Revert "Revert "Fixed windows compatability. This required"" This reverts commit 384da6d2f4bc81e684ccc2563941c6f8d344f7d7. --- src/find/matchers/mod.rs | 22 ++++--- src/find/matchers/perm.rs | 21 +++++-- src/find/matchers/printer.rs | 4 +- src/find/matchers/time.rs | 5 +- src/find/mod.rs | 107 +++++++++++++++++++++----------- src/testing/commandline/main.rs | 9 ++- tests/common/test_helpers.rs | 14 ++++- tests/exec_unit_tests.rs | 14 ++--- tests/find_exec_tests.rs | 12 ++-- 9 files changed, 139 insertions(+), 69 deletions(-) diff --git a/src/find/matchers/mod.rs b/src/find/matchers/mod.rs index 0f13959..d412edd 100644 --- a/src/find/matchers/mod.rs +++ b/src/find/matchers/mod.rs @@ -375,6 +375,7 @@ fn build_matcher_tree(args: &[&str], mod tests { use walkdir::{DirEntry, WalkDir}; use find::Config; + use find::tests::fix_up_slashes; use find::tests::FakeDependencies; use super::*; @@ -384,7 +385,7 @@ mod tests { /// probably be a string starting with "test_data/" (cargo's tests run with /// a working directory set to the root findutils folder). pub fn get_dir_entry_for(directory: &str, filename: &str) -> DirEntry { - for wrapped_dir_entry in WalkDir::new(directory) { + for wrapped_dir_entry in WalkDir::new(fix_up_slashes(directory)) { let dir_entry = wrapped_dir_entry.unwrap(); if dir_entry.file_name().to_string_lossy() == filename { return dir_entry; @@ -404,7 +405,8 @@ mod tests { assert!(matcher.matches(&abbbc_lower, &mut deps.new_matcher_io())); assert!(!matcher.matches(&abbbc_upper, &mut deps.new_matcher_io())); - assert_eq!(deps.get_output_as_string(), "./test_data/simple/abbbc\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/simple/abbbc\n")); } #[test] @@ -419,7 +421,7 @@ mod tests { assert!(matcher.matches(&abbbc_lower, &mut deps.new_matcher_io())); assert!(matcher.matches(&abbbc_upper, &mut deps.new_matcher_io())); assert_eq!(deps.get_output_as_string(), - "./test_data/simple/abbbc\n./test_data/simple/subdir/ABBBC\n"); + fix_up_slashes("./test_data/simple/abbbc\n./test_data/simple/subdir/ABBBC\n")); } #[test] @@ -433,7 +435,8 @@ mod tests { .unwrap(); assert!(matcher.matches(&abbbc_lower, &mut deps.new_matcher_io())); - assert_eq!(deps.get_output_as_string(), "./test_data/simple/abbbc\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/simple/abbbc\n")); } } @@ -521,7 +524,8 @@ mod tests { // build a matcher using an explicit -a argument let matcher = build_top_level_matcher(&["-true", "-a", "-true"], &mut config).unwrap(); assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); - assert_eq!(deps.get_output_as_string(), "./test_data/simple/abbbc\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/simple/abbbc\n")); } #[test] @@ -536,7 +540,8 @@ mod tests { let matcher = build_top_level_matcher(args, &mut config).unwrap(); assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); - assert_eq!(deps.get_output_as_string(), "./test_data/simple/abbbc\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/simple/abbbc\n")); } let mut config = Config::default(); @@ -567,7 +572,8 @@ mod tests { let matcher = build_top_level_matcher(&["-true", "-true"], &mut config).unwrap(); assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); - assert_eq!(deps.get_output_as_string(), "./test_data/simple/abbbc\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/simple/abbbc\n")); } #[test] @@ -583,7 +589,7 @@ mod tests { assert!(!matcher.matches(&abbbc, &mut deps.new_matcher_io())); // two print matchers means doubled output assert_eq!(deps.get_output_as_string(), - "./test_data/simple/abbbc\n./test_data/simple/abbbc\n"); + fix_up_slashes("./test_data/simple/abbbc\n./test_data/simple/abbbc\n")); } #[test] diff --git a/src/find/matchers/perm.rs b/src/find/matchers/perm.rs index defad4b..90cedc3 100644 --- a/src/find/matchers/perm.rs +++ b/src/find/matchers/perm.rs @@ -10,6 +10,7 @@ use std::error::Error; use std::io::{stderr, Write}; +#[cfg(unix)] use std::str::FromStr; use walkdir::DirEntry; @@ -17,6 +18,7 @@ use find::matchers::{Matcher, MatcherIO}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[cfg(unix)] pub enum ComparisonType { /// mode bits have to match exactly Exact, @@ -27,6 +29,7 @@ pub enum ComparisonType { AnyOf, } +#[cfg(unix)] impl FromStr for ComparisonType { type Err = Box; fn from_str(s: &str) -> Result> { @@ -43,6 +46,7 @@ impl FromStr for ComparisonType { } } +#[cfg(unix)] impl ComparisonType { fn mode_bits_match(&self, pattern: u32, value: u32) -> bool { match *self { @@ -53,6 +57,7 @@ impl ComparisonType { } } +#[cfg(unix)] mod parsing { use regex::Regex; use std::error::Error; @@ -255,11 +260,15 @@ mod parsing { } } +#[cfg(unix)] pub struct PermMatcher { pattern: u32, comparison_type: ComparisonType, } +#[cfg(not(unix))] +pub struct PermMatcher {} + impl PermMatcher { #[cfg(unix)] pub fn new(pattern: &str) -> Result> { @@ -271,8 +280,8 @@ impl PermMatcher { } #[cfg(not(unix))] - pub fn new(pattern: &str) -> Result> { - Err(From("Permission matching is not available on this platform")) + pub fn new(_dummy_pattern: &str) -> Result> { + Err(From::from("Permission matching is not available on this platform")) } pub fn new_box(pattern: &str) -> Result, Box> { @@ -300,14 +309,17 @@ impl Matcher for PermMatcher { } #[cfg(not(unix))] - fn matches(&self, file_info: &DirEntry, _: &mut MatcherIO) -> bool { - stderr().write("Permission matching not available on this platform!"); + fn matches(&self, _dummy_file_info: &DirEntry, _: &mut MatcherIO) -> bool { + writeln!(&mut stderr(), + "Permission matching not available on this platform!") + .unwrap(); return false; } } #[cfg(test)] +#[cfg(unix)] mod tests { use find::matchers::Matcher; use find::matchers::tests::get_dir_entry_for; @@ -475,7 +487,6 @@ mod tests { } #[test] - #[cfg(unix)] fn perm_matches() { let file_info = get_dir_entry_for("test_data/simple", "abbbc"); let deps = FakeDependencies::new(); diff --git a/src/find/matchers/printer.rs b/src/find/matchers/printer.rs index 1a67d47..1926315 100644 --- a/src/find/matchers/printer.rs +++ b/src/find/matchers/printer.rs @@ -41,6 +41,7 @@ mod tests { use find::matchers::tests::get_dir_entry_for; use find::matchers::Matcher; use find::tests::FakeDependencies; + use find::tests::fix_up_slashes; use super::*; #[test] @@ -50,6 +51,7 @@ mod tests { let matcher = Printer::new(); let deps = FakeDependencies::new(); assert!(matcher.matches(&abbbc, &mut deps.new_matcher_io())); - assert_eq!("./test_data/simple/abbbc\n", deps.get_output_as_string()); + assert_eq!(fix_up_slashes("./test_data/simple/abbbc\n"), + deps.get_output_as_string()); } } diff --git a/src/find/matchers/time.rs b/src/find/matchers/time.rs index 5b63b9c..62e4f55 100644 --- a/src/find/matchers/time.rs +++ b/src/find/matchers/time.rs @@ -6,7 +6,7 @@ use std; use std::error::Error; -use std::fs::{File, Metadata}; +use std::fs::{self, Metadata}; use std::io::{stderr, Write}; use std::time::SystemTime; use walkdir::DirEntry; @@ -22,8 +22,7 @@ pub struct NewerMatcher { impl NewerMatcher { pub fn new(path_to_file: &str) -> Result> { - let f = File::open(path_to_file)?; - let metadata = f.metadata()?; + let metadata = fs::metadata(path_to_file)?; Ok(NewerMatcher { given_modification_time: metadata.modified()? }) } diff --git a/src/find/mod.rs b/src/find/mod.rs index f1c5e58..2374974 100644 --- a/src/find/mod.rs +++ b/src/find/mod.rs @@ -219,6 +219,18 @@ mod tests { use super::*; + #[cfg(windows)] + /// Windows-only bodge for converting between path separators. + pub fn fix_up_slashes(path: &str) -> String { + path.replace("/", "\\") + } + + #[cfg(not(windows))] + /// Do nothing equivalent of the above for non-windows systems. + pub fn fix_up_slashes(path: &str) -> String { + path.to_string() + } + /// A struct that implements Dependencies, but uses faked implementations, /// allowing us to check output, set the time returned by clocks etc. pub struct FakeDependencies { @@ -267,14 +279,15 @@ mod tests { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/simple", "-sorted"], &deps); + let rc = find_main(&["find", &fix_up_slashes("./test_data/simple"), "-sorted"], + &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/simple\n\ + fix_up_slashes("./test_data/simple\n\ ./test_data/simple/abbbc\n\ ./test_data/simple/subdir\n\ - ./test_data/simple/subdir/ABBBC\n"); + ./test_data/simple/subdir/ABBBC\n")); } #[test] @@ -282,46 +295,53 @@ mod tests { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/simple", "-sorted", "-depth"], &deps); + let rc = find_main(&["find", &fix_up_slashes("./test_data/simple"), "-sorted", "-depth"], + &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/simple/abbbc\n\ + fix_up_slashes("./test_data/simple/abbbc\n\ ./test_data/simple/subdir/ABBBC\n\ ./test_data/simple/subdir\n\ - ./test_data/simple\n"); + ./test_data/simple\n")); } #[test] fn find_maxdepth() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-sorted", "-maxdepth", "2"], - &deps); + let rc = + find_main(&["find", &fix_up_slashes("./test_data/depth"), "-sorted", "-maxdepth", "2"], + &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/depth\n\ + fix_up_slashes("./test_data/depth\n\ ./test_data/depth/1\n\ ./test_data/depth/1/2\n\ ./test_data/depth/1/f1\n\ - ./test_data/depth/f0\n"); + ./test_data/depth/f0\n")); } #[test] fn find_maxdepth_depth_first() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-sorted", "-maxdepth", "2", "-depth"], + let rc = find_main(&["find", + &fix_up_slashes("./test_data/depth"), + "-sorted", + "-maxdepth", + "2", + "-depth"], &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/depth/1/2\n\ + fix_up_slashes("./test_data/depth/1/2\n\ ./test_data/depth/1/f1\n\ ./test_data/depth/1\n\ ./test_data/depth/f0\n\ - ./test_data/depth\n"); + ./test_data/depth\n")); } #[test] @@ -329,7 +349,7 @@ mod tests { let deps = FakeDependencies::new(); let rc = find_main(&["find", - "./test_data/depth", + &fix_up_slashes("./test_data/depth"), "-sorted", "-print", ",", @@ -340,54 +360,64 @@ mod tests { assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/depth\n\ + fix_up_slashes("./test_data/depth\n\ ./test_data/depth/1\n\ - ./test_data/depth/f0\n"); + ./test_data/depth/f0\n")); } #[test] fn find_zero_maxdepth() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-maxdepth", "0"], &deps); + let rc = find_main(&["find", &fix_up_slashes("./test_data/depth"), "-maxdepth", "0"], + &deps); assert_eq!(rc, 0); - assert_eq!(deps.get_output_as_string(), "./test_data/depth\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/depth\n")); } #[test] fn find_zero_maxdepth_depth_first() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-maxdepth", "0", "-depth"], - &deps); + let rc = + find_main(&["find", &fix_up_slashes("./test_data/depth"), "-maxdepth", "0", "-depth"], + &deps); assert_eq!(rc, 0); - assert_eq!(deps.get_output_as_string(), "./test_data/depth\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/depth\n")); } #[test] fn find_mindepth() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-sorted", "-mindepth", "3"], - &deps); + let rc = + find_main(&["find", &fix_up_slashes("./test_data/depth"), "-sorted", "-mindepth", "3"], + &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/depth/1/2/3\n\ + fix_up_slashes("./test_data/depth/1/2/3\n\ ./test_data/depth/1/2/3/f3\n\ - ./test_data/depth/1/2/f2\n"); + ./test_data/depth/1/2/f2\n")); } #[test] fn find_mindepth_depth_first() { let deps = FakeDependencies::new(); - let rc = find_main(&["find", "./test_data/depth", "-sorted", "-mindepth", "3", "-depth"], + let rc = find_main(&["find", + &fix_up_slashes("./test_data/depth"), + "-sorted", + "-mindepth", + "3", + "-depth"], &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/depth/1/2/3/f3\n\ + fix_up_slashes("./test_data/depth/1/2/3/f3\n\ ./test_data/depth/1/2/3\n\ - ./test_data/depth/1/2/f2\n"); + ./test_data/depth/1/2/f2\n")); } #[test] @@ -402,7 +432,7 @@ mod tests { let rc = find_main(&["find", &new_dir.path().to_string_lossy(), "-newer", - "./test_data/simple/abbbc"], + &fix_up_slashes("./test_data/simple/abbbc")], &deps); assert_eq!(rc, 0); @@ -412,7 +442,7 @@ mod tests { // now do it the other way around, and nothing should be output let deps = FakeDependencies::new(); let rc = find_main(&["find", - "./test_data/simple/abbbc", + &fix_up_slashes("./test_data/simple/abbbc"), "-newer", &new_dir.path().to_string_lossy()], &deps); @@ -461,12 +491,17 @@ mod tests { let mut deps = FakeDependencies::new(); deps.set_time(file_time); - let rc = find_main(&["find", "./test_data/simple/subdir", "-type", "f", arg, "0"], + let rc = find_main(&["find", + &fix_up_slashes("./test_data/simple/subdir"), + "-type", + "f", + arg, + "0"], &deps); assert_eq!(rc, 0); assert_eq!(deps.get_output_as_string(), - "./test_data/simple/subdir/ABBBC\n"); + fix_up_slashes("./test_data/simple/subdir/ABBBC\n")); } // now Check file time doesn't match a file that's too new @@ -487,11 +522,13 @@ mod tests { let deps = FakeDependencies::new(); // only look at files because the "size" of a directory is a system (and filesystem) // dependent thing and we want these tests to be universal. - let rc = find_main(&["find", "./test_data/size", "-type", "f", "-size", "1b"], - &deps); + let rc = + find_main(&["find", &fix_up_slashes("./test_data/size"), "-type", "f", "-size", "1b"], + &deps); assert_eq!(rc, 0); - assert_eq!(deps.get_output_as_string(), "./test_data/size/512bytes\n"); + assert_eq!(deps.get_output_as_string(), + fix_up_slashes("./test_data/size/512bytes\n")); let deps = FakeDependencies::new(); let rc = find_main(&["find", "./test_data/size", "-type", "f", "-size", "+1b"], diff --git a/src/testing/commandline/main.rs b/src/testing/commandline/main.rs index bfb0b73..2d813ff 100644 --- a/src/testing/commandline/main.rs +++ b/src/testing/commandline/main.rs @@ -62,10 +62,13 @@ fn main() { // first two args are going to be the path to this executable and // the destination_dir we want to write to. Don't write either of those // as they'll be non-deterministic. - f.write_fmt(format_args!("cwd={}\nargs={:?}\n", - env::current_dir().unwrap().to_string_lossy(), - &args[2..])) + f.write_fmt(format_args!("cwd={}\nargs=\n", + env::current_dir().unwrap().to_string_lossy())) .expect("failed to write to file"); + for arg in &args[2..] { + f.write_fmt(format_args!("{}\n", arg)).expect("failed to write to file"); + } + } std::process::exit(if config.exit_with_failure { 2 } else { 0 }); } diff --git a/tests/common/test_helpers.rs b/tests/common/test_helpers.rs index 11dc332..4c2680a 100644 --- a/tests/common/test_helpers.rs +++ b/tests/common/test_helpers.rs @@ -71,12 +71,24 @@ pub fn path_to_testing_commandline() -> String { .to_string() } +#[cfg(windows)] +/// A copy of find::tests::fix_up_slashes. +/// TODO: find out how to share #[cfg(test)] functions/structs between unit +/// and integration tests. +pub fn fix_up_slashes(path: &str) -> String { + path.replace("/", "\\") +} + +#[cfg(not(windows))] +pub fn fix_up_slashes(path: &str) -> String { + path.to_string() +} /// A copy of find::tests::FakeDependencies. /// TODO: find out how to share #[cfg(test)] functions/structs between unit /// and integration tests. pub fn get_dir_entry_for(directory: &str, filename: &str) -> DirEntry { - for wrapped_dir_entry in WalkDir::new(directory) { + for wrapped_dir_entry in WalkDir::new(fix_up_slashes(directory)) { let dir_entry = wrapped_dir_entry.unwrap(); if dir_entry.file_name().to_string_lossy() == filename { return dir_entry; diff --git a/tests/exec_unit_tests.rs b/tests/exec_unit_tests.rs index 7f4e920..f4a6152 100644 --- a/tests/exec_unit_tests.rs +++ b/tests/exec_unit_tests.rs @@ -44,8 +44,8 @@ fn matching_executes_code() { let mut s = String::new(); f.read_to_string(&mut s).expect("failed to read output file"); assert_eq!(s, - format!("cwd={}\nargs=[\"abc\", \"test_data/simple/abbbc\", \"xyz\"]\n", - env::current_dir().unwrap().to_string_lossy())); + fix_up_slashes(&format!("cwd={}\nargs=\nabc\ntest_data/simple/abbbc\nxyz\n", + env::current_dir().unwrap().to_string_lossy()))); } #[test] @@ -66,8 +66,8 @@ fn matching_executes_code_in_files_directory() { let mut s = String::new(); f.read_to_string(&mut s).expect("failed to read output file"); assert_eq!(s, - format!("cwd={}/test_data/simple\nargs=[\"abc\", \"./abbbc\", \"xyz\"]\n", - env::current_dir().unwrap().to_string_lossy())); + fix_up_slashes(&format!("cwd={}/test_data/simple\nargs=\nabc\n./abbbc\nxyz\n", + env::current_dir().unwrap().to_string_lossy()))); } #[test] @@ -92,7 +92,7 @@ fn matching_fails_if_executable_fails() { let mut s = String::new(); f.read_to_string(&mut s).expect("failed to read output file"); assert_eq!(s, - format!("cwd={}/test_data/simple\nargs=[\"--exit_with_failure\", \"abc\", \ - \"./abbbc\", \"xyz\"]\n", - env::current_dir().unwrap().to_string_lossy())); + fix_up_slashes(&format!("cwd={}/test_data/simple\nargs=\n--exit_with_failure\nabc\n.\ + /abbbc\nxyz\n", + env::current_dir().unwrap().to_string_lossy()))); } diff --git a/tests/find_exec_tests.rs b/tests/find_exec_tests.rs index 2443519..f24e434 100644 --- a/tests/find_exec_tests.rs +++ b/tests/find_exec_tests.rs @@ -31,7 +31,7 @@ fn find_exec() { let deps = FakeDependencies::new(); let rc = find_main(&["find", - "./test_data/simple/subdir", + &fix_up_slashes("./test_data/simple/subdir"), "-type", "f", "-exec", @@ -53,8 +53,8 @@ fn find_exec() { let mut s = String::new(); f.read_to_string(&mut s).expect("failed to read output file"); assert_eq!(s, - format!("cwd={}\nargs=[\"(\", \"./test_data/simple/subdir/ABBBC\", \"-o\"]\n", - env::current_dir().unwrap().to_string_lossy())); + fix_up_slashes(&format!("cwd={}\nargs=\n(\n./test_data/simple/subdir/ABBBC\n-o\n", + env::current_dir().unwrap().to_string_lossy()))); } #[test] @@ -65,7 +65,7 @@ fn find_execdir() { // only look at files because the "size" of a directory is a system (and filesystem) // dependent thing and we want these tests to be universal. let rc = find_main(&["find", - "./test_data/simple/subdir", + &fix_up_slashes("./test_data/simple/subdir"), "-type", "f", "-execdir", @@ -87,7 +87,7 @@ fn find_execdir() { let mut s = String::new(); f.read_to_string(&mut s).expect("failed to read output file"); assert_eq!(s, - format!("cwd={}/test_data/simple/subdir\nargs=[\")\", \"./ABBBC\", \",\"]\n", - env::current_dir().unwrap().to_string_lossy())); + fix_up_slashes(&format!("cwd={}/test_data/simple/subdir\nargs=\n)\n./ABBBC\n,\n", + env::current_dir().unwrap().to_string_lossy()))); }