From 8309a1f00654cdb2ac4a0a21be67052cc87ac660 Mon Sep 17 00:00:00 2001
From: Laurence Tratt <laurie@tratt.net>
Date: Tue, 26 Nov 2019 14:36:49 +0000
Subject: [PATCH] Fix bug where the outer `num_ignored` variable was ignored.

The basic problem is that Rust allows move closure to capture variables if they
have a `Copy`able type, leading to the "outer" and "inner" variables to diverge.
For example, this simple program:

  fn main() {
    let mut x = 0;
    let mut y = move |z: u32| {
      x += z;
      println!("inner x {}", x);
    };

    println!("outer x {}", x);
    y(2);
    println!("outer x {}", x);
  }

outputs:

  outer x 0
  inner x 2
  outer x 0

Note that the above program gives no warnings (or errors) in rustc or clippy.

In this case, the `num_ignored` variable had started life in single-threaded
code and later been moved to multi-threaded code. It compiled, even though the
outer variable could never be mutated. In other words, lang_tester always said
that 0 tests were ignored -- even if some were ignored!

This commit fixes this problem using an `AtomicUsize` so that ignored tests now
count as can be seen here:

  $ cargo run --example rust_lang_tester
      Finished dev [unoptimized + debuginfo] target(s) in 0.12s
       Running `target/debug/examples/rust_lang_tester`

  running 7 tests
  test lang_tests::no_main ... ok
  test lang_tests::ignore ... ignored
  test lang_tests::unknown_var ... ok
  test lang_tests::exit_code ... ok
  test lang_tests::unused_var ... ok
  test lang_tests::sig_caught ... ok
  test lang_tests::custom_cla ... ok

  test result: ok. 7 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out
---
 src/tester.rs | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/tester.rs b/src/tester.rs
index 7f59cbe..e79fd9c 100644
--- a/src/tester.rs
+++ b/src/tester.rs
@@ -7,7 +7,10 @@ use std::{
     path::{Path, PathBuf},
     process::{self, Command, ExitStatus},
     str,
-    sync::{Arc, Mutex},
+    sync::{
+        atomic::{AtomicUsize, Ordering},
+        Arc, Mutex,
+    },
     thread::sleep,
     time::{Duration, Instant},
 };
@@ -460,11 +463,12 @@ fn test_file(
     inner: Arc<LangTesterPooler>,
 ) -> (Vec<(String, TestFailure)>, usize) {
     let failures = Arc::new(Mutex::new(Vec::new()));
-    let mut num_ignored = 0;
+    let num_ignored = Arc::new(AtomicUsize::new(0));
     let pool = ThreadPool::new(inner.test_threads);
     for p in test_files {
         let test_fname = p.file_stem().unwrap().to_str().unwrap().to_owned();
 
+        let num_ignored = num_ignored.clone();
         let failures = failures.clone();
         let inner = inner.clone();
         pool.execute(move || {
@@ -479,26 +483,26 @@ fn test_file(
 
             if test_str.is_empty() {
                 write_ignored(test_fname.as_str(), "test string is empty", inner);
-                num_ignored += 1;
+                num_ignored.fetch_add(1, Ordering::Relaxed);
                 return;
             }
 
             let tests = parse_tests(&test_str);
             if (inner.ignored && !tests.ignore) || (!inner.ignored && tests.ignore) {
                 write_ignored(test_fname.as_str(), "", inner);
-                num_ignored += 1;
+                num_ignored.fetch_add(1, Ordering::Relaxed);
                 return;
             }
 
             if run_tests(Arc::clone(&inner), tests.tests, p, failures) {
-                num_ignored += 1;
+                num_ignored.fetch_add(1, Ordering::Relaxed);
             }
         });
     }
     pool.join();
     let failures = Mutex::into_inner(Arc::try_unwrap(failures).unwrap()).unwrap();
 
-    (failures, num_ignored)
+    (failures, Arc::try_unwrap(num_ignored).unwrap().into_inner())
 }
 
 /// Run the tests for `path`.