Skip to content

Commit

Permalink
Add --fix-expected to help batch fix to expected tests (Internal to…
Browse files Browse the repository at this point in the history
…ol only) (rust-lang#2228)

This option will basically override the expected file with Kani's output for every test that fails the expectation. Developers should still prune the expected file to ensure only the information that is needed gets pushed.

My motivation is that I always find myself updating expected tests manually by rerunning Kani for each failure, piping the output to the expected file and edit the file after. With this hack, I only need to do the last step manually.

Since we still fail the test, the files that were updated are listed, which makes it very easy to track down the changes.
  • Loading branch information
celinval authored Feb 22, 2023
1 parent c1e19d3 commit 375f821
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 20 deletions.
5 changes: 5 additions & 0 deletions tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ pub struct Config {

/// Whether we will run the tests or not.
pub dry_run: bool,

/// Whether we should update expected tests when there is a mismatch. This is helpful for
/// updating multiple tests. Users should still manually edit the files after to only keep
/// relevant expectations.
pub fix_expected: bool,
}

#[derive(Debug, Clone)]
Expand Down
4 changes: 4 additions & 0 deletions tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optopt("", "timeout", "the timeout for each test in seconds", "TIMEOUT")
.optflag("", "no-fail-fast", "run all tests regardless of failure")
.optflag("", "dry-run", "don't actually run the tests")
.optflag("", "fix-expected",
"override all expected files that did not match the output. Tests will NOT fail when there is a mismatch")
;

let (argv0, args_) = args.split_first().unwrap();
Expand Down Expand Up @@ -160,6 +162,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
force_rerun: matches.opt_present("force-rerun"),
fail_fast: !matches.opt_present("no-fail-fast"),
dry_run: matches.opt_present("dry-run"),
fix_expected: matches.opt_present("fix-expected"),
timeout,
}
}
Expand All @@ -180,6 +183,7 @@ pub fn log_config(config: &Config) {
logv(c, format!("timeout: {:?}", config.timeout));
logv(c, format!("fail-fast: {:?}", config.fail_fast));
logv(c, format!("dry-run: {:?}", config.dry_run));
logv(c, format!("fix-expected: {:?}", config.fix_expected));
logv(
c,
format!(
Expand Down
53 changes: 33 additions & 20 deletions tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{fatal_error, json};

use std::env;
use std::fs::{self, create_dir_all};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::{Command, ExitStatus, Output, Stdio};
use std::str;

Expand Down Expand Up @@ -279,8 +279,7 @@ impl<'test> TestCx<'test> {
}

let proc_res = self.compose_and_run(cargo);
let expected = fs::read_to_string(self.testpaths.file.clone()).unwrap();
self.verify_output(&proc_res, &expected);
self.verify_output(&proc_res, &self.testpaths.file);

// TODO: We should probably be checking the exit status somehow
// See https://github.com/model-checking/kani/issues/1895
Expand Down Expand Up @@ -334,7 +333,7 @@ impl<'test> TestCx<'test> {
}

// Check if the `expected` file exists, and load its contents into `expected_output`
let expected_output = if let Some(expected_path) = exec_config.expected {
let expected_path = if let Some(expected_path) = exec_config.expected {
let expected_rel_path = PathBuf::from(expected_path);
let expected_path = self.testpaths.file.join(expected_rel_path);
if !expected_path.exists() {
Expand All @@ -344,7 +343,7 @@ impl<'test> TestCx<'test> {
);
fatal_error(&err_msg);
}
Some(fs::read_to_string(expected_path).unwrap())
Some(expected_path)
} else {
None
};
Expand All @@ -355,8 +354,8 @@ impl<'test> TestCx<'test> {
let proc_res = self.compose_and_run(script_path_cmd);

// Compare with expected output if it was provided
if let Some(output) = expected_output {
self.verify_output(&proc_res, &output);
if let Some(path) = expected_path {
self.verify_output(&proc_res, &path);
}

// Compare with exit code (0 if it wasn't provided)
Expand All @@ -376,9 +375,8 @@ impl<'test> TestCx<'test> {
/// the expected output in `expected` file.
fn run_expected_test(&self) {
let proc_res = self.run_kani();
let expected =
fs::read_to_string(self.testpaths.file.parent().unwrap().join("expected")).unwrap();
self.verify_output(&proc_res, &expected);
let expected_path = self.testpaths.file.parent().unwrap().join("expected");
self.verify_output(&proc_res, &expected_path);
}

/// Runs Kani with stub implementations of various data structures.
Expand All @@ -397,20 +395,35 @@ impl<'test> TestCx<'test> {

/// Print an error if the verification output does not contain the expected
/// lines.
fn verify_output(&self, proc_res: &ProcRes, expected: &str) {
fn verify_output(&self, proc_res: &ProcRes, expected_path: &Path) {
// Include the output from stderr here for cases where there are exceptions
let expected = fs::read_to_string(expected_path).unwrap();
let output = proc_res.stdout.to_string() + &proc_res.stderr;
if let Some(lines) = TestCx::contains_lines(
let diff = TestCx::contains_lines(
&output.split('\n').collect::<Vec<_>>(),
expected.split('\n').collect(),
) {
self.fatal_proc_rec(
&format!(
"test failed: expected output to contain the line(s):\n{}",
lines.join("\n")
),
proc_res,
);
);
match (diff, self.config.fix_expected) {
(None, _) => { /* Test passed. Do nothing*/ }
(Some(_), true) => {
// Fix output but still fail the test so users know which ones were updated
fs::write(expected_path, output)
.expect(&format!("Failed to update file {}", expected_path.display()));
self.fatal_proc_rec(
&format!("updated `{}` file, please review", expected_path.display()),
proc_res,
)
}
(Some(lines), false) => {
// Throw an error
self.fatal_proc_rec(
&format!(
"test failed: expected output to contain the line(s):\n{}",
lines.join("\n")
),
proc_res,
);
}
}
}

Expand Down

0 comments on commit 375f821

Please sign in to comment.