From f312e2c5f58d7bde403510a900f41b849131b3da Mon Sep 17 00:00:00 2001 From: Amaan Qureshi Date: Sun, 1 Sep 2024 04:32:10 -0400 Subject: [PATCH] fix(test): retain attributes when running `test -u` (cherry picked from commit 272ebf77b9e2b24a0ba1e30599794a77e434c159) --- cli/src/test.rs | 106 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 18 deletions(-) diff --git a/cli/src/test.rs b/cli/src/test.rs index a3bf5e88..cd570053 100644 --- a/cli/src/test.rs +++ b/cli/src/test.rs @@ -339,7 +339,7 @@ fn run_tests( opts: &mut TestOptions, mut indent_level: i32, failures: &mut Vec<(String, String, String)>, - corrected_entries: &mut Vec<(String, String, String, usize, usize)>, + corrected_entries: &mut Vec<(String, String, String, String, usize, usize)>, has_parse_errors: &mut bool, ) -> Result { match test_entry { @@ -350,6 +350,7 @@ fn run_tests( header_delim_len, divider_delim_len, has_fields, + attributes_str, attributes, } => { print!("{}", " ".repeat(indent_level as usize)); @@ -389,7 +390,32 @@ fn run_tests( opts.test_num, paint(opts.color.then_some(AnsiColor::Green), &name) ); + if opts.update { + let input = String::from_utf8(input.clone()).unwrap(); + let output = format_sexp(&output, 0); + corrected_entries.push(( + name.clone(), + input, + output, + attributes_str.clone(), + header_delim_len, + divider_delim_len, + )); + } } else { + if opts.update { + let input = String::from_utf8(input.clone()).unwrap(); + // Keep the original `expected` output if the actual output has no error + let output = format_sexp(&output, 0); + corrected_entries.push(( + name.clone(), + input, + output, + attributes_str.clone(), + header_delim_len, + divider_delim_len, + )); + } println!( "{:>3}.  {}", opts.test_num, @@ -424,6 +450,7 @@ fn run_tests( name.clone(), input, output, + attributes_str.clone(), header_delim_len, divider_delim_len, )); @@ -447,6 +474,7 @@ fn run_tests( name.clone(), input, expected_output, + attributes_str.clone(), header_delim_len, divider_delim_len, )); @@ -455,6 +483,7 @@ fn run_tests( name.clone(), input, actual_output, + attributes_str.clone(), header_delim_len, divider_delim_len, )); @@ -584,7 +613,7 @@ fn count_subtests(test_entry: &TestEntry) -> usize { fn write_tests( file_path: &Path, - corrected_entries: &[(String, String, String, usize, usize)], + corrected_entries: &[(String, String, String, String, usize, usize)], ) -> Result<()> { let mut buffer = fs::File::create(file_path)?; write_tests_to_buffer(&mut buffer, corrected_entries) @@ -592,9 +621,9 @@ fn write_tests( fn write_tests_to_buffer( buffer: &mut impl Write, - corrected_entries: &[(String, String, String, usize, usize)], + corrected_entries: &[(String, String, String, String, usize, usize)], ) -> Result<()> { - for (i, (name, input, output, header_delim_len, divider_delim_len)) in + for (i, (name, input, output, attributes_str, header_delim_len, divider_delim_len)) in corrected_entries.iter().enumerate() { if i > 0 { @@ -602,8 +631,13 @@ fn write_tests_to_buffer( } writeln!( buffer, - "{}\n{name}\n{}\n{input}\n{}\n\n{}", + "{}\n{name}\n{}{}\n{input}\n{}\n\n{}", "=".repeat(*header_delim_len), + if attributes_str.is_empty() { + attributes_str.clone() + } else { + format!("{}\n", attributes_str) + }, "=".repeat(*header_delim_len), "-".repeat(*divider_delim_len), output.trim() @@ -661,6 +695,7 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - let mut children = Vec::new(); let bytes = content.as_bytes(); let mut prev_name = String::new(); + let mut prev_attributes_str = String::new(); let mut prev_header_end = 0; // Find the first test header in the file, and determine if it has a @@ -691,17 +726,20 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - .map_or("".as_bytes(), |m| m.as_bytes()); let mut test_name = String::new(); + let mut attributes_str = String::new(); + let mut seen_marker = false; - for line in str::from_utf8(test_name_and_markers) - .unwrap() - .lines() + let test_name_and_markers = str::from_utf8(test_name_and_markers).unwrap(); + for line in test_name_and_markers + .split_inclusive('\n') .filter(|s| !s.is_empty()) { - match line.split('(').next().unwrap() { + let trimmed_line = line.trim(); + match trimmed_line.split('(').next().unwrap() { ":skip" => (seen_marker, skip) = (true, true), ":platform" => { - if let Some(platforms) = line.strip_prefix(':').and_then(|s| { + if let Some(platforms) = trimmed_line.strip_prefix(':').and_then(|s| { s.strip_prefix("platform(") .and_then(|s| s.strip_suffix(')')) }) { @@ -714,7 +752,7 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - ":fail-fast" => (seen_marker, fail_fast) = (true, true), ":error" => (seen_marker, error) = (true, true), ":language" => { - if let Some(lang) = line.strip_prefix(':').and_then(|s| { + if let Some(lang) = trimmed_line.strip_prefix(':').and_then(|s| { s.strip_prefix("language(") .and_then(|s| s.strip_suffix(')')) }) { @@ -724,11 +762,11 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - } _ if !seen_marker => { test_name.push_str(line); - test_name.push('\n'); } _ => {} } } + attributes_str.push_str(test_name_and_markers.strip_prefix(&test_name).unwrap()); // prefer skip over error, both shouldn't be set if skip { @@ -747,10 +785,16 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - } else { Some(test_name.trim_end().to_string()) }; + let attributes_str = if attributes_str.is_empty() { + None + } else { + Some(attributes_str.trim_end().to_string()) + }; Some(( header_delim_len, header_range, test_name, + attributes_str, TestAttributes { skip, platform: platform.unwrap_or(true), @@ -765,12 +809,15 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - }); let (mut prev_header_len, mut prev_attributes) = (80, TestAttributes::default()); - for (header_delim_len, header_range, test_name, attributes) in header_matches.chain(Some(( - 80, - bytes.len()..bytes.len(), - None, - TestAttributes::default(), - ))) { + for (header_delim_len, header_range, test_name, attributes_str, attributes) in header_matches + .chain(Some(( + 80, + bytes.len()..bytes.len(), + None, + None, + TestAttributes::default(), + ))) + { // Find the longest line of dashes following each test description. That line // separates the input from the expected output. Ignore any matches whose suffix // does not match the first suffix in the file. @@ -822,6 +869,7 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - header_delim_len: prev_header_len, divider_delim_len, has_fields, + attributes_str: prev_attributes_str, attributes: prev_attributes, }; @@ -831,6 +879,7 @@ fn parse_test_content(name: String, content: &str, file_path: Option) - } prev_attributes = attributes; prev_name = test_name.unwrap_or_default(); + prev_attributes_str = attributes_str.unwrap_or_default(); prev_header_len = header_delim_len; prev_header_end = header_range.end; } @@ -884,6 +933,7 @@ d header_delim_len: 15, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -893,6 +943,7 @@ d header_delim_len: 16, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, ], @@ -943,6 +994,7 @@ abc header_delim_len: 18, divider_delim_len: 7, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -952,6 +1004,7 @@ abc header_delim_len: 25, divider_delim_len: 19, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, ], @@ -1017,6 +1070,7 @@ abc "title 1".to_string(), "input 1".to_string(), "output 1".to_string(), + String::new(), 80, 80, ), @@ -1024,6 +1078,7 @@ abc "title 2".to_string(), "input 2".to_string(), "output 2".to_string(), + String::new(), 80, 80, ), @@ -1104,6 +1159,7 @@ code header_delim_len: 18, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -1113,6 +1169,7 @@ code header_delim_len: 18, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -1122,6 +1179,7 @@ code header_delim_len: 25, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), } ], @@ -1195,6 +1253,7 @@ NOT A TEST HEADER header_delim_len: 18, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -1204,6 +1263,7 @@ NOT A TEST HEADER header_delim_len: 18, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -1213,6 +1273,7 @@ NOT A TEST HEADER header_delim_len: 25, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), } ], @@ -1258,6 +1319,7 @@ code with ---- header_delim_len: 15, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), }, TestEntry::Example { @@ -1267,6 +1329,7 @@ code with ---- header_delim_len: 20, divider_delim_len: 3, has_fields: false, + attributes_str: String::new(), attributes: TestAttributes::default(), } ] @@ -1304,6 +1367,7 @@ a header_delim_len: 21, divider_delim_len: 3, has_fields: false, + attributes_str: ":skip".to_string(), attributes: TestAttributes { skip: true, platform: true, @@ -1360,6 +1424,7 @@ a header_delim_len: 25, divider_delim_len: 3, has_fields: false, + attributes_str: format!(":platform({})\n:fail-fast", std::env::consts::OS), attributes: TestAttributes { skip: false, platform: true, @@ -1375,6 +1440,11 @@ a header_delim_len: 29, divider_delim_len: 3, has_fields: false, + attributes_str: if std::env::consts::OS == "linux" { + ":platform(macos)\n:language(foo)".to_string() + } else { + ":platform(linux)\n:language(foo)".to_string() + }, attributes: TestAttributes { skip: false, platform: false,