Skip to content

Commit b3947b4

Browse files
committed
Return errors on first entry of WalkDir
The first entry of WalkDir is being skipped - the problem with this is that the first entry is where WalkDir can report that the directory for comparison doesn't exist. The current code will return `false` for the following cases: dir_diff::is_different("dir_that_exists", "dir_that_does_not_exist"); dir_diff::is_different("dir_that_does_not_exist", "also_does_not_exist"); This is obviously not ideal since the directory not existing is probably not what the user intended. This commit fixes up the issue by checking the first entry returned from WalkDir for error instead of skipping it. This causes `is_different` to return an error if either or both directories don't actually exist. Signed-off-by: Nick Stevens <[email protected]>
1 parent ea5b7fc commit b3947b4

File tree

2 files changed

+28
-8
lines changed

2 files changed

+28
-8
lines changed

src/lib.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ pub enum Error {
3838
/// assert!(dir_diff::is_different("dir/a", "dir/b").unwrap());
3939
/// ```
4040
pub fn is_different<A: AsRef<Path>, B: AsRef<Path>>(a_base: A, b_base: B) -> Result<bool, Error> {
41-
let mut a_walker = walk_dir(a_base);
42-
let mut b_walker = walk_dir(b_base);
41+
let mut a_walker = walk_dir(a_base)?;
42+
let mut b_walker = walk_dir(b_base)?;
4343

4444
for (a, b) in (&mut a_walker).zip(&mut b_walker) {
4545
let a = a?;
@@ -53,14 +53,16 @@ pub fn is_different<A: AsRef<Path>, B: AsRef<Path>>(a_base: A, b_base: B) -> Res
5353
}
5454
}
5555

56-
Ok(!a_walker.next().is_none() || !b_walker.next().is_none())
56+
Ok(a_walker.next().is_some() || b_walker.next().is_some())
5757
}
5858

59-
fn walk_dir<P: AsRef<Path>>(path: P) -> std::iter::Skip<walkdir::IntoIter> {
60-
WalkDir::new(path)
61-
.sort_by(compare_by_file_name)
62-
.into_iter()
63-
.skip(1)
59+
fn walk_dir<P: AsRef<Path>>(path: P) -> Result<walkdir::IntoIter, std::io::Error> {
60+
let mut walkdir = WalkDir::new(path).sort_by(compare_by_file_name).into_iter();
61+
if let Some(Err(e)) = walkdir.next() {
62+
Err(e.into())
63+
} else {
64+
Ok(walkdir)
65+
}
6466
}
6567

6668
fn compare_by_file_name(a: &DirEntry, b: &DirEntry) -> Ordering {

tests/smoke.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,24 @@ fn oneempty() {
3434
assert!(dir_diff::is_different("tests/oneempty/dir1", "tests/oneempty/dir2").unwrap());
3535
}
3636

37+
#[test]
38+
#[should_panic]
39+
fn firstmissing() {
40+
assert!(dir_diff::is_different("does_not_exist", "tests/easy/good/dir1").unwrap());
41+
}
42+
43+
#[test]
44+
#[should_panic]
45+
fn secondmissing() {
46+
assert!(dir_diff::is_different("tests/easy/good/dir1", "does_not_exist").unwrap());
47+
}
48+
49+
#[test]
50+
#[should_panic]
51+
fn bothmissing() {
52+
assert!(dir_diff::is_different("does_not_exist", "also_does_not_exist").unwrap());
53+
}
54+
3755
#[test]
3856
fn reflexive() {
3957
assert!(dir_diff::is_different("tests/reflexive/dir1", "tests/reflexive/dir2").unwrap());

0 commit comments

Comments
 (0)