Add --check summary of failures

Add a warning to stderr at the end similar to other *sum utilities when --check is used and some files failed verification.

At least the last part of the comment above check_one_checkfile seemed to be incorrect so I removed that comment.

Fixes #283

2 files changed, 30 insertions, 20 deletions

diff --git a/b3sum/src/main.rs b/b3sum/src/main.rs
index 91a87a1..11aac5a 100644
--- a/b3sum/src/main.rs
+++ b/b3sum/src/main.rs
@@ -490,8 +490,8 @@ fn hash_one_input(path: &Path, args: &Args) -> Result<()> {
 }
 
 // Returns true for success. Having a boolean return value here, instead of
-// passing down the some_file_failed reference, makes it less likely that we
-// might forget to set it in some error condition.
+// passing down the files_failed reference, makes it less likely that we might
+// forget to set it in some error condition.
 fn check_one_line(line: &str, args: &Args) -> bool {
     let parse_result = parse_check_line(&line);
     let ParsedCheckLine {
@@ -537,7 +537,7 @@ fn check_one_line(line: &str, args: &Args) -> bool {
     }
 }
 
-fn check_one_checkfile(path: &Path, args: &Args, some_file_failed: &mut bool) -> Result<()> {
+fn check_one_checkfile(path: &Path, args: &Args, files_failed: &mut i64) -> Result<()> {
     let checkfile_input = Input::open(path, args)?;
     let mut bufreader = io::BufReader::new(checkfile_input);
     let mut line = String::new();
@@ -551,7 +551,7 @@ fn check_one_checkfile(path: &Path, args: &Args, some_file_failed: &mut bool) ->
         // return, so it doesn't return a Result.
         let success = check_one_line(&line, args);
         if !success {
-            *some_file_failed = true;
+            *files_failed += 1;
         }
     }
 }
@@ -564,16 +564,11 @@ fn main() -> Result<()> {
     }
     let thread_pool = thread_pool_builder.build()?;
     thread_pool.install(|| {
-        let mut some_file_failed = false;
+        let mut files_failed = 0i64;
         // Note that file_args automatically includes `-` if nothing is given.
         for path in &args.file_args {
             if args.check() {
-                // A hash mismatch or a failure to read a hashed file will be
-                // printed in the checkfile loop, and will not propagate here.
-                // This is similar to the explicit error handling we do in the
-                // hashing case immediately below. In these cases,
-                // some_file_failed will be set to false.
-                check_one_checkfile(path, &args, &mut some_file_failed)?;
+                check_one_checkfile(path, &args, &mut files_failed)?;
             } else {
                 // Errors encountered in hashing are tolerated and printed to
                 // stderr. This allows e.g. `b3sum *` to print errors for
@@ -581,12 +576,15 @@ fn main() -> Result<()> {
                 // errors we'll still return non-zero at the end.
                 let result = hash_one_input(path, &args);
                 if let Err(e) = result {
-                    some_file_failed = true;
+                    files_failed += 1;
                     eprintln!("{}: {}: {}", NAME, path.to_string_lossy(), e);
                 }
             }
         }
-        std::process::exit(if some_file_failed { 1 } else { 0 });
+        if args.check() && files_failed > 0 {
+            eprintln!("{}: WARNING {} computed checksum{} did NOT match", NAME, files_failed, if files_failed == 1 {""} else {"s"});
+        }
+        std::process::exit(if files_failed > 0 { 1 } else { 0 });
     })
 }
 
diff --git a/b3sum/tests/cli_tests.rs b/b3sum/tests/cli_tests.rs
index 6b0ca5a..9b9b9de 100644
--- a/b3sum/tests/cli_tests.rs
+++ b/b3sum/tests/cli_tests.rs
@@ -420,7 +420,7 @@ fn test_check() {
         c/d: OK\n";
     assert!(!output.status.success());
     assert_eq!(expected_check_failure, stdout);
-    assert_eq!("", stderr);
+    assert_eq!("b3sum: WARNING 1 computed checksum did NOT match\n", stderr);
 
     // Delete one of the files and check again.
     fs::remove_file(dir.path().join("b")).unwrap();
@@ -442,7 +442,7 @@ fn test_check() {
     );
     assert!(!output.status.success());
     assert_eq!(expected_check_failure, stdout);
-    assert_eq!("", stderr);
+    assert_eq!("b3sum: WARNING 1 computed checksum did NOT match\n", stderr);
 
     // Confirm that --quiet suppresses the OKs but not the FAILEDs.
     let output = cmd!(b3sum_exe(), "--check", "--quiet", &checkfile_path)
@@ -457,7 +457,7 @@ fn test_check() {
     let expected_check_failure = format!("b: FAILED ({})\n", open_file_error);
     assert!(!output.status.success());
     assert_eq!(expected_check_failure, stdout);
-    assert_eq!("", stderr);
+    assert_eq!("b3sum: WARNING 1 computed checksum did NOT match\n", stderr);
 }
 
 #[test]
@@ -472,9 +472,12 @@ fn test_check_invalid_characters() {
         .unwrap();
     let stdout = std::str::from_utf8(&output.stdout).unwrap();
     let stderr = std::str::from_utf8(&output.stderr).unwrap();
+    let expected_stderr = "\
+        b3sum: Null character in path\n\
+        b3sum: WARNING 1 computed checksum did NOT match\n";
     assert!(!output.status.success());
     assert_eq!("", stdout);
-    assert_eq!("b3sum: Null character in path\n", stderr);
+    assert_eq!(expected_stderr, stderr);
 
     // Check that a Unicode replacement character in the path fails.
     let output = cmd!(b3sum_exe(), "--check")
@@ -486,9 +489,12 @@ fn test_check_invalid_characters() {
         .unwrap();
     let stdout = std::str::from_utf8(&output.stdout).unwrap();
     let stderr = std::str::from_utf8(&output.stderr).unwrap();
+    let expected_stderr = "\
+        b3sum: Unicode replacement character in path\n\
+        b3sum: WARNING 1 computed checksum did NOT match\n";
     assert!(!output.status.success());
     assert_eq!("", stdout);
-    assert_eq!("b3sum: Unicode replacement character in path\n", stderr);
+    assert_eq!(expected_stderr, stderr);
 
     // Check that an invalid escape sequence in the path fails.
     let output = cmd!(b3sum_exe(), "--check")
@@ -500,9 +506,12 @@ fn test_check_invalid_characters() {
         .unwrap();
     let stdout = std::str::from_utf8(&output.stdout).unwrap();
     let stderr = std::str::from_utf8(&output.stderr).unwrap();
+    let expected_stderr = "\
+        b3sum: Invalid backslash escape\n\
+        b3sum: WARNING 1 computed checksum did NOT match\n";
     assert!(!output.status.success());
     assert_eq!("", stdout);
-    assert_eq!("b3sum: Invalid backslash escape\n", stderr);
+    assert_eq!(expected_stderr, stderr);
 
     // Windows also forbids literal backslashes. Check for that if and only if
     // we're on Windows.
@@ -516,9 +525,12 @@ fn test_check_invalid_characters() {
             .unwrap();
         let stdout = std::str::from_utf8(&output.stdout).unwrap();
         let stderr = std::str::from_utf8(&output.stderr).unwrap();
+        let expected_stderr = "\
+            b3sum: Backslash in path\n\
+            b3sum: WARNING 1 computed checksum did NOT match\n";
         assert!(!output.status.success());
         assert_eq!("", stdout);
-        assert_eq!("b3sum: Backslash in path\n", stderr);
+        assert_eq!(expected_stderr, stderr);
     }
 }