From b0a18cbadf6cca712471179778bc13bcfb56aef0 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 3 Jul 2023 19:38:55 -0400 Subject: [PATCH] Update debuginfo test runner to provide more useful output This change makes debuginfo tests more user friendly. Changes: - Print all lines that fail to match the patterns instead of just the first - Provide better error messages that also say what did match - Strip leading whitespace from directives so they are not skipped if indented - Improve documentation and improve nesting on some related items As an example, given the following debuginfo test with intentional fails: ```rust // from tests/debuginfo/rc_arc.rs // cdb-command:dx rc,d // cdb-check:rc,d : 111 [Type: alloc::rc::Rc] // cdb-check: [Reference count] : 11 [Type: core::cell FAIL::Cell] // cdb-check: [Weak reference count] : 2 [Type: core::cell FAIL::Cell] // ... ``` The current output (tested in #113313) only shows the first mismatch: ``` 2023-07-04T08:10:00.1939267Z ---- [debuginfo-cdb] tests\debuginfo\rc_arc.rs stdout ---- 2023-07-04T08:10:00.1942182Z 2023-07-04T08:10:00.1957463Z error: line not found in debugger output: [Reference count] : 11 [Type: core::cell FAIL::Cell] 2023-07-04T08:10:00.1958272Z status: exit code: 0 ``` With this change, you are able to see all failures in that check group, as well as what parts were successful. The output is now: ``` 2023-07-04T09:45:57.2514224Z error: check directive(s) from `C:\a\rust\rust\tests\debuginfo\rc_arc.rs` not found in debugger output. errors: 2023-07-04T09:45:57.2514631Z (rc_arc.rs:31) ` [Reference count] : 11 [Type: core::cell FAIL::Cell]` 2023-07-04T09:45:57.2514908Z (rc_arc.rs:32) ` [Weak reference count] : 2 [Type: core::cell FAIL::Cell]` 2023-07-04T09:45:57.2515181Z (rc_arc.rs:41) ` [Reference count] : 21 [Type: core::sync::atomic FAIL::AtomicUsize]` 2023-07-04T09:45:57.2515452Z (rc_arc.rs:50) `dyn_rc,d [Type: alloc::rc::Rc >]` 2023-07-04T09:45:57.2515695Z the following subset of check directive(s) was found successfully:: 2023-07-04T09:45:57.2516080Z (rc_arc.rs:30) `rc,d : 111 [Type: alloc::rc::Rc]` 2023-07-04T09:45:57.2516312Z (rc_arc.rs:35) `weak_rc,d : 111 [Type: alloc::rc::Weak]` 2023-07-04T09:45:57.2516555Z (rc_arc.rs:36) ` [Reference count] : 11 [Type: core::cell::Cell]` 2023-07-04T09:45:57.2516881Z (rc_arc.rs:37) ` [Weak reference count] : 2 [Type: core::cell::Cell]` ... ``` Which makes it easier to see what did and didn't succeed without manual comparison against the source test file. --- src/tools/compiletest/src/header.rs | 13 +- src/tools/compiletest/src/runtest.rs | 78 ++++----- src/tools/compiletest/src/runtest/debugger.rs | 150 +++++++++++------- 3 files changed, 132 insertions(+), 109 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index c835962ad12..ad10c3e07ce 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -588,21 +588,22 @@ pub fn local_pass_mode(&self) -> Option { } } +/// Extract a `(Option, directive)` directive from a line if comment is present. pub fn line_directive<'line>( comment: &str, ln: &'line str, ) -> Option<(Option<&'line str>, &'line str)> { + let ln = ln.trim_start(); if ln.starts_with(comment) { let ln = ln[comment.len()..].trim_start(); if ln.starts_with('[') { // A comment like `//[foo]` is specific to revision `foo` - if let Some(close_brace) = ln.find(']') { - let lncfg = &ln[1..close_brace]; + let Some(close_brace) = ln.find(']') else { + panic!("malformed condition directive: expected `{}[foo]`, found `{}`", comment, ln); + }; - Some((Some(lncfg), ln[(close_brace + 1)..].trim_start())) - } else { - panic!("malformed condition directive: expected `{}[foo]`, found `{}`", comment, ln) - } + let lncfg = &ln[1..close_brace]; + Some((Some(lncfg), ln[(close_brace + 1)..].trim_start())) } else { Some((None, ln)) } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index fddfbac78d5..37a0c1a084a 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -41,7 +41,7 @@ use crate::is_android_gdb_target; mod debugger; -use debugger::{check_debugger_output, DebuggerCommands}; +use debugger::DebuggerCommands; #[cfg(test)] mod tests; @@ -997,16 +997,13 @@ fn run_debuginfo_cdb_test_no_opt(&self) { }; // Parse debugger commands etc from test files - let DebuggerCommands { commands, check_lines, breakpoint_lines, .. } = - match DebuggerCommands::parse_from( - &self.testpaths.file, - self.config, - prefixes, - self.revision, - ) { - Ok(cmds) => cmds, - Err(e) => self.fatal(&e), - }; + let dbg_cmds = DebuggerCommands::parse_from( + &self.testpaths.file, + self.config, + prefixes, + self.revision, + ) + .unwrap_or_else(|e| self.fatal(&e)); // https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands let mut script_str = String::with_capacity(2048); @@ -1023,12 +1020,12 @@ fn run_debuginfo_cdb_test_no_opt(&self) { // Set breakpoints on every line that contains the string "#break" let source_file_name = self.testpaths.file.file_name().unwrap().to_string_lossy(); - for line in &breakpoint_lines { + for line in &dbg_cmds.breakpoint_lines { script_str.push_str(&format!("bp `{}:{}`\n", source_file_name, line)); } // Append the other `cdb-command:`s - for line in &commands { + for line in &dbg_cmds.commands { script_str.push_str(line); script_str.push_str("\n"); } @@ -1058,7 +1055,7 @@ fn run_debuginfo_cdb_test_no_opt(&self) { self.fatal_proc_rec("Error while running CDB", &debugger_run_result); } - if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + if let Err(e) = dbg_cmds.check_output(&debugger_run_result) { self.fatal_proc_rec(&e, &debugger_run_result); } } @@ -1088,17 +1085,14 @@ fn run_debuginfo_gdb_test_no_opt(&self) { PREFIXES }; - let DebuggerCommands { commands, check_lines, breakpoint_lines } = - match DebuggerCommands::parse_from( - &self.testpaths.file, - self.config, - prefixes, - self.revision, - ) { - Ok(cmds) => cmds, - Err(e) => self.fatal(&e), - }; - let mut cmds = commands.join("\n"); + let dbg_cmds = DebuggerCommands::parse_from( + &self.testpaths.file, + self.config, + prefixes, + self.revision, + ) + .unwrap_or_else(|e| self.fatal(&e)); + let mut cmds = dbg_cmds.commands.join("\n"); // compile test file (it should have 'compile-flags:-g' in the header) let should_run = self.run_if_enabled(); @@ -1132,13 +1126,14 @@ fn run_debuginfo_gdb_test_no_opt(&self) { ./{}/stage2/lib/rustlib/{}/lib/\n", self.config.host, self.config.target )); - for line in &breakpoint_lines { + for line in &dbg_cmds.breakpoint_lines { script_str.push_str( - &format!( + format!( "break {:?}:{}\n", self.testpaths.file.file_name().unwrap().to_string_lossy(), *line - )[..], + ) + .as_str(), ); } script_str.push_str(&cmds); @@ -1279,7 +1274,7 @@ fn run_debuginfo_gdb_test_no_opt(&self) { } // Add line breakpoints - for line in &breakpoint_lines { + for line in &dbg_cmds.breakpoint_lines { script_str.push_str(&format!( "break '{}':{}\n", self.testpaths.file.file_name().unwrap().to_string_lossy(), @@ -1315,7 +1310,7 @@ fn run_debuginfo_gdb_test_no_opt(&self) { self.fatal_proc_rec("gdb failed to execute", &debugger_run_result); } - if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + if let Err(e) = dbg_cmds.check_output(&debugger_run_result) { self.fatal_proc_rec(&e, &debugger_run_result); } } @@ -1372,16 +1367,13 @@ fn run_debuginfo_lldb_test_no_opt(&self) { }; // Parse debugger commands etc from test files - let DebuggerCommands { commands, check_lines, breakpoint_lines, .. } = - match DebuggerCommands::parse_from( - &self.testpaths.file, - self.config, - prefixes, - self.revision, - ) { - Ok(cmds) => cmds, - Err(e) => self.fatal(&e), - }; + let dbg_cmds = DebuggerCommands::parse_from( + &self.testpaths.file, + self.config, + prefixes, + self.revision, + ) + .unwrap_or_else(|e| self.fatal(&e)); // Write debugger script: // We don't want to hang when calling `quit` while the process is still running @@ -1430,7 +1422,7 @@ fn run_debuginfo_lldb_test_no_opt(&self) { // Set breakpoints on every line that contains the string "#break" let source_file_name = self.testpaths.file.file_name().unwrap().to_string_lossy(); - for line in &breakpoint_lines { + for line in &dbg_cmds.breakpoint_lines { script_str.push_str(&format!( "breakpoint set --file '{}' --line {}\n", source_file_name, line @@ -1438,7 +1430,7 @@ fn run_debuginfo_lldb_test_no_opt(&self) { } // Append the other commands - for line in &commands { + for line in &dbg_cmds.commands { script_str.push_str(line); script_str.push_str("\n"); } @@ -1458,7 +1450,7 @@ fn run_debuginfo_lldb_test_no_opt(&self) { self.fatal_proc_rec("Error while running LLDB", &debugger_run_result); } - if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + if let Err(e) = dbg_cmds.check_output(&debugger_run_result) { self.fatal_proc_rec(&e, &debugger_run_result); } } diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs index 379ff0bab40..eebe5f3580b 100644 --- a/src/tools/compiletest/src/runtest/debugger.rs +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -2,18 +2,25 @@ use crate::header::line_directive; use crate::runtest::ProcRes; +use std::fmt::Write; use std::fs::File; use std::io::{BufRead, BufReader}; -use std::path::Path; +use std::path::{Path, PathBuf}; +/// Representation of information to invoke a debugger and check its output pub(super) struct DebuggerCommands { + /// Commands for the debuuger pub commands: Vec, - pub check_lines: Vec, + /// Lines to insert breakpoints at pub breakpoint_lines: Vec, + /// Contains the source line number to check and the line itself + check_lines: Vec<(usize, String)>, + /// Source file name + file: PathBuf, } impl DebuggerCommands { - pub(super) fn parse_from( + pub fn parse_from( file: &Path, config: &Config, debugger_prefixes: &[&str], @@ -21,7 +28,7 @@ pub(super) fn parse_from( ) -> Result { let directives = debugger_prefixes .iter() - .map(|prefix| (format!("{}-command", prefix), format!("{}-check", prefix))) + .map(|prefix| (format!("{prefix}-command"), format!("{prefix}-check"))) .collect::>(); let mut breakpoint_lines = vec![]; @@ -29,63 +36,88 @@ pub(super) fn parse_from( let mut check_lines = vec![]; let mut counter = 0; let reader = BufReader::new(File::open(file).unwrap()); - for line in reader.lines() { + for (line_no, line) in reader.lines().enumerate() { counter += 1; - match line { - Ok(line) => { - let (lnrev, line) = line_directive("//", &line).unwrap_or((None, &line)); + let line = line.map_err(|e| format!("Error while parsing debugger commands: {}", e))?; + let (lnrev, line) = line_directive("//", &line).unwrap_or((None, &line)); - // Skip any revision specific directive that doesn't match the current - // revision being tested - if lnrev.is_some() && lnrev != rev { - continue; - } + // Skip any revision specific directive that doesn't match the current + // revision being tested + if lnrev.is_some() && lnrev != rev { + continue; + } - if line.contains("#break") { - breakpoint_lines.push(counter); - } + if line.contains("#break") { + breakpoint_lines.push(counter); + } - for &(ref command_directive, ref check_directive) in &directives { - config - .parse_name_value_directive(&line, command_directive) - .map(|cmd| commands.push(cmd)); + for &(ref command_directive, ref check_directive) in &directives { + config + .parse_name_value_directive(&line, command_directive) + .map(|cmd| commands.push(cmd)); - config - .parse_name_value_directive(&line, check_directive) - .map(|cmd| check_lines.push(cmd)); - } - } - Err(e) => return Err(format!("Error while parsing debugger commands: {}", e)), + config + .parse_name_value_directive(&line, check_directive) + .map(|cmd| check_lines.push((line_no, cmd))); } } - Ok(Self { commands, check_lines, breakpoint_lines }) - } -} - -pub(super) fn check_debugger_output( - debugger_run_result: &ProcRes, - check_lines: &[String], -) -> Result<(), String> { - let num_check_lines = check_lines.len(); - - let mut check_line_index = 0; - for line in debugger_run_result.stdout.lines() { - if check_line_index >= num_check_lines { - break; - } - - if check_single_line(line, &(check_lines[check_line_index])[..]) { - check_line_index += 1; - } - } - if check_line_index != num_check_lines && num_check_lines > 0 { - Err(format!("line not found in debugger output: {}", check_lines[check_line_index])) - } else { - Ok(()) + Ok(Self { commands, breakpoint_lines, check_lines, file: file.to_owned() }) + } + + /// Given debugger output and lines to check, ensure that every line is + /// contained in the debugger output. The check lines need to be found in + /// order, but there can be extra lines between. + pub fn check_output(&self, debugger_run_result: &ProcRes) -> Result<(), String> { + // (src_lineno, ck_line) that we did find + let mut found = vec![]; + // (src_lineno, ck_line) that we couldn't find + let mut missing = vec![]; + // We can find our any current match anywhere after our last match + let mut last_idx = 0; + let dbg_lines: Vec<&str> = debugger_run_result.stdout.lines().collect(); + + for (src_lineno, ck_line) in &self.check_lines { + if let Some(offset) = dbg_lines + .iter() + .skip(last_idx) + .position(|out_line| check_single_line(out_line, &ck_line)) + { + last_idx += offset; + found.push((src_lineno, dbg_lines[last_idx])); + } else { + missing.push((src_lineno, ck_line)); + } + } + + if missing.is_empty() { + Ok(()) + } else { + let fname = self.file.file_name().unwrap().to_string_lossy(); + let mut msg = format!( + "check directive(s) from `{}` not found in debugger output. errors:", + self.file.display() + ); + + for (src_lineno, err_line) in missing { + write!(msg, "\n ({fname}:{num}) `{err_line}`", num = src_lineno + 1).unwrap(); + } + + if !found.is_empty() { + let init = "\nthe following subset of check directive(s) was found successfully:"; + msg.push_str(init); + for (src_lineno, found_line) in found { + write!(msg, "\n ({fname}:{num}) `{found_line}`", num = src_lineno + 1) + .unwrap(); + } + } + + Err(msg) + } } } +/// Check that the pattern in `check_line` applies to `line`. Returns `true` if they do match. fn check_single_line(line: &str, check_line: &str) -> bool { // Allow check lines to leave parts unspecified (e.g., uninitialized // bits in the wrong case of an enum) with the notation "[...]". @@ -101,21 +133,19 @@ fn check_single_line(line: &str, check_line: &str) -> bool { } let (mut rest, first_fragment) = if can_start_anywhere { - match line.find(check_fragments[0]) { - Some(pos) => (&line[pos + check_fragments[0].len()..], 1), - None => return false, - } + let Some(pos) = line.find(check_fragments[0]) else { + return false; + }; + (&line[pos + check_fragments[0].len()..], 1) } else { (line, 0) }; for current_fragment in &check_fragments[first_fragment..] { - match rest.find(current_fragment) { - Some(pos) => { - rest = &rest[pos + current_fragment.len()..]; - } - None => return false, - } + let Some(pos) = rest.find(current_fragment) else { + return false; + }; + rest = &rest[pos + current_fragment.len()..]; } if !can_end_anywhere && !rest.is_empty() { false } else { true }