Skip to content

Commit

Permalink
Merge pull request #195 from oli-obk/filtering
Browse files Browse the repository at this point in the history
Fix filtered test reporting
  • Loading branch information
oli-obk authored Jan 24, 2024
2 parents 750637c + 477eff6 commit 815e26d
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

* report an error instead of panicking when encountering a suggestion that does not belong to the main file.
* number of filtered tests is now > 0 when things actually got filtered.

### Changed

Expand Down
28 changes: 17 additions & 11 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,10 @@ pub fn run_tests(mut config: Config) -> Result<()> {

/// The filter used by `run_tests` to only run on `.rs` files that are
/// specified by [`Config::filter_files`] and [`Config::skip_files`].
pub fn default_file_filter(path: &Path, config: &Config) -> bool {
path.extension().is_some_and(|ext| ext == "rs") && default_any_file_filter(path, config)
/// Returns `None` if there is no extension or the extension is not `.rs`.
pub fn default_file_filter(path: &Path, config: &Config) -> Option<bool> {
path.extension().filter(|&ext| ext == "rs")?;
Some(default_any_file_filter(path, config))
}

/// Run on all files that are specified by [`Config::filter_files`] and
Expand Down Expand Up @@ -203,8 +205,6 @@ pub enum TestOk {
Ok,
/// The test was ignored due to a rule (`//@only-*` or `//@ignore-*`)
Ignored,
/// The test was filtered with the `file_filter` argument.
Filtered,
}

/// The possible results a single test can have.
Expand Down Expand Up @@ -233,9 +233,12 @@ struct TestRun {
/// All `configs` are being run in parallel.
/// If multiple configs are provided, the [`Config::threads`] value of the first one is used;
/// the thread count of all other configs is ignored.
/// The file filter is supposed to return `None` if it was filtered because of file extensions
/// and `Some(false)` if the file was rejected out of other reasons like the file path not matching
/// a user defined filter.
pub fn run_tests_generic(
mut configs: Vec<Config>,
file_filter: impl Fn(&Path, &Config) -> bool + Sync,
file_filter: impl Fn(&Path, &Config) -> Option<bool> + Sync,
per_file_config: impl Fn(&mut Config, &Path, &[u8]) + Sync,
status_emitter: impl StatusEmitter + Send,
) -> Result<()> {
Expand Down Expand Up @@ -275,6 +278,7 @@ pub fn run_tests_generic(
},
};

let mut filtered = 0;
run_and_collect(
num_threads,
|submit| {
Expand All @@ -297,10 +301,14 @@ pub fn run_tests_generic(
for entry in entries {
todo.push_back((entry, config));
}
} else if file_filter(&path, config) {
let status = status_emitter.register_test(path);
// Forward .rs files to the test workers.
submit.send((status, config)).unwrap();
} else if let Some(matched) = file_filter(&path, config) {
if matched {
let status = status_emitter.register_test(path);
// Forward .rs files to the test workers.
submit.send((status, config)).unwrap();
} else {
filtered += 1;
}
}
}
},
Expand Down Expand Up @@ -357,13 +365,11 @@ pub fn run_tests_generic(
let mut failures = vec![];
let mut succeeded = 0;
let mut ignored = 0;
let mut filtered = 0;

for run in results {
match run.result {
Ok(TestOk::Ok) => succeeded += 1,
Ok(TestOk::Ignored) => ignored += 1,
Ok(TestOk::Filtered) => filtered += 1,
Err(errored) => failures.push((run.status, errored)),
}
}
Expand Down
1 change: 0 additions & 1 deletion src/status_emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ impl TestStatus for TextTest {
Ok(TestOk::Ok) => "ok".green(),
Err(Errored { .. }) => "FAILED".bright_red().bold(),
Ok(TestOk::Ignored) => "ignored (in-test comment)".yellow(),
Ok(TestOk::Filtered) => return,
};
let old_msg = self.msg();
let msg = format!("... {result}");
Expand Down
40 changes: 22 additions & 18 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,29 @@ fn main() -> Result<()> {
.ends_with("-fail");
if cfg!(windows) && path.components().any(|c| c.as_os_str() == "basic-bin") {
// on windows there's also a .pdb file, so we get additional errors that aren't there on other platforms
return false;
return Some(false);
}
path.ends_with("Cargo.toml")
&& path.parent().unwrap().parent().unwrap() == root_dir
&& match config
.comment_defaults
.base_immut()
.mode
.as_deref()
.unwrap()
{
Mode::Pass => !fail,
// This is weird, but `cargo test` returns 101 instead of 1 when
// multiple [[test]]s exist. If there's only one test, it returns
// 1 on failure.
Mode::Panic => fail,
Mode::Run { .. } | Mode::Yolo { .. } | Mode::Fail { .. } => unreachable!(),
}
&& default_any_file_filter(path, config)
if !path.ends_with("Cargo.toml") {
return None;
}
Some(
path.parent().unwrap().parent().unwrap() == root_dir
&& match config
.comment_defaults
.base_immut()
.mode
.as_deref()
.unwrap()
{
Mode::Pass => !fail,
// This is weird, but `cargo test` returns 101 instead of 1 when
// multiple [[test]]s exist. If there's only one test, it returns
// 1 on failure.
Mode::Panic => fail,
Mode::Run { .. } | Mode::Yolo { .. } | Mode::Fail { .. } => unreachable!(),
}
&& default_any_file_filter(path, config),
)
},
|_, _, _| {},
(
Expand Down

0 comments on commit 815e26d

Please sign in to comment.