Skip to content

Commit

Permalink
fix(cli): don't update tests automatically if parse errors are detected
Browse files Browse the repository at this point in the history
  • Loading branch information
amaanq committed Feb 17, 2024
1 parent 464dbef commit 98e8815
Showing 1 changed file with 32 additions and 9 deletions.
41 changes: 32 additions & 9 deletions cli/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::util;
use ansi_term::Colour;
use anyhow::{anyhow, Context, Result};
use difference::{Changeset, Difference};
use indoc::indoc;
use lazy_static::lazy_static;
use regex::bytes::{Regex as ByteRegex, RegexBuilder as ByteRegexBuilder};
use regex::Regex;
Expand Down Expand Up @@ -84,13 +85,15 @@ pub fn run_tests_at_path(parser: &mut Parser, opts: &mut TestOptions) -> Result<

let mut failures = Vec::new();
let mut corrected_entries = Vec::new();
let mut has_parse_errors = false;
run_tests(
parser,
test_entry,
opts,
0,
&mut failures,
&mut corrected_entries,
&mut has_parse_errors,
)?;

parser.stop_printing_dot_graphs();
Expand All @@ -100,7 +103,7 @@ pub fn run_tests_at_path(parser: &mut Parser, opts: &mut TestOptions) -> Result<
} else {
println!();

if opts.update {
if opts.update && !has_parse_errors {
if failures.len() == 1 {
println!("1 update:\n");
} else {
Expand All @@ -110,12 +113,17 @@ pub fn run_tests_at_path(parser: &mut Parser, opts: &mut TestOptions) -> Result<
for (i, (name, ..)) in failures.iter().enumerate() {
println!(" {}. {name}", i + 1);
}

Ok(())
} else {
if failures.len() == 1 {
println!("1 failure:");
} else {
println!("{} failures:", failures.len());
has_parse_errors = opts.update && has_parse_errors;

if !has_parse_errors {
if failures.len() == 1 {
println!("1 failure:");
} else {
println!("{} failures:", failures.len());
}
}

print_diff_key();
Expand All @@ -125,7 +133,14 @@ pub fn run_tests_at_path(parser: &mut Parser, opts: &mut TestOptions) -> Result<
let expected = format_sexp_indented(expected, 2);
print_diff(&actual, &expected);
}
Err(anyhow!(""))

if has_parse_errors {
Err(anyhow!(indoc! {"
Some tests failed to parse with unexpected `ERROR` or `MISSING` nodes, as shown above, and cannot be updated automatically.
Either fix the grammar or manually update the tests if this is expected."}))
} else {
Err(anyhow!(""))
}
}
}
}
Expand Down Expand Up @@ -153,8 +168,7 @@ pub fn check_queries_at_path(language: &Language, path: &Path) -> Result<()> {

pub fn print_diff_key() {
println!(
"\n{} / {} / {}",
Colour::White.paint("correct"),
"\ncorrect / {} / {}",
Colour::Green.paint("expected"),
Colour::Red.paint("unexpected")
);
Expand Down Expand Up @@ -185,6 +199,7 @@ fn run_tests(
mut indent_level: i32,
failures: &mut Vec<(String, String, String)>,
corrected_entries: &mut Vec<(String, String, String, usize, usize)>,
has_parse_errors: &mut bool,
) -> Result<()> {
match test_entry {
TestEntry::Example {
Expand Down Expand Up @@ -218,6 +233,13 @@ fn run_tests(
if opts.update {
let input = String::from_utf8(input).unwrap();
let output = format_sexp(&actual);

// Only bail early before updating if the actual is not the output, sometimes
// users want to test cases that are intended to have errors, hence why this
// check isn't shown above
if actual.contains("ERROR") || actual.contains("MISSING") {
*has_parse_errors = true;
}
corrected_entries.push((
name.clone(),
input,
Expand Down Expand Up @@ -278,11 +300,12 @@ fn run_tests(
indent_level,
failures,
corrected_entries,
has_parse_errors,
)?;
}

if let Some(file_path) = file_path {
if opts.update && failures.len() - failure_count > 0 {
if opts.update && failures.len() - failure_count > 0 && !*has_parse_errors {
write_tests(&file_path, corrected_entries)?;
}
corrected_entries.clear();
Expand Down

0 comments on commit 98e8815

Please sign in to comment.