Skip to content

Commit

Permalink
Fix bug where the outer num_ignored variable was ignored.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ltratt committed Nov 26, 2019
1 parent a0a78e4 commit 8309a1f
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand Down Expand Up @@ -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 || {
Expand All @@ -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`.
Expand Down

0 comments on commit 8309a1f

Please sign in to comment.