Rollup merge of #144805 - Zalathar:proc-res, r=jieyouxu

compiletest: Preliminary cleanup of `ProcRes` printing/unwinding
While experimenting with changes to how compiletest handles output capture, error reporting, and unwinding, I repeatedly ran in to difficulties with this core code for reporting test failures caused by a subprocess.
There should be no change in compiletest output.
r? jieyouxu

Author: samueltardieu

src/tools/compiletest/src/runtest.rs

@@ -354,13 +354,10 @@ impl<'test> TestCx<'test> {
 }
 }else{
 if proc_res.status.success(){
-{
-self.error(&format!("{} test did not emit an error",self.config.mode));
-ifself.config.mode == crate::common::TestMode::Ui{
-println!("note: by default, ui tests are expected not to compile");
-}
- proc_res.fatal(None, || ());
-};
+let err = &format!("{} test did not emit an error",self.config.mode);
+let extra_note = (self.config.mode == crate::common::TestMode::Ui)
+.then_some("note: by default, ui tests are expected not to compile");
+self.fatal_proc_rec_general(err, extra_note, proc_res, || ());
 }
 
 if !self.props.dont_check_failure_status{
@@ -606,7 +603,10 @@ impl<'test> TestCx<'test> {
 );
 }else{
 for pattern in missing_patterns {
-self.error(&format!("error pattern '{}' not found!", pattern));
+println!(
+"\n{prefix}: error pattern '{pattern}' not found!",
+ prefix = self.error_prefix()
+);
 }
 self.fatal_proc_rec("multiple error patterns not found", proc_res);
 }
@@ -824,10 +824,11 @@ impl<'test> TestCx<'test> {
 // - only known line - meh, but suggested
 // - others are not worth suggesting
 if !unexpected.is_empty(){
-self.error(&format!(
-"{} diagnostics reported in JSON output but not expected in test file",
- unexpected.len(),
-));
+println!(
+"\n{prefix}: {n} diagnostics reported in JSON output but not expected in test file",
+ prefix = self.error_prefix(),
+ n = unexpected.len(),
+);
 for error in&unexpected {
 print_error(error);
 letmut suggestions = Vec::new();
@@ -857,10 +858,11 @@ impl<'test> TestCx<'test> {
 }
 }
 if !not_found.is_empty(){
-self.error(&format!(
-"{} diagnostics expected in test file but not reported in JSON output",
- not_found.len()
-));
+println!(
+"\n{prefix}: {n} diagnostics expected in test file but not reported in JSON output",
+ prefix = self.error_prefix(),
+ n = not_found.len(),
+);
 for error in&not_found {
 print_error(error);
 letmut suggestions = Vec::new();
@@ -1995,33 +1997,52 @@ impl<'test> TestCx<'test> {
 output_base_name(self.config,self.testpaths,self.safe_revision())
 }
 
-fnerror(&self,err:&str){
+/// Prefix to print before error messages. Normally just `error`, but also
+/// includes the revision name for tests that use revisions.
+#[must_use]
+fnerror_prefix(&self) -> String{
 matchself.revision{
-Some(rev) => println!("\nerror in revision `{}`: {}", rev, err),
-None => println!("\nerror: {}", err),
+Some(rev) => format!("error in revision `{rev}`"),
+None => format!("error"),
 }
 }
 
 #[track_caller]
 fnfatal(&self,err:&str) -> ! {
-self.error(err);
+println!("\n{prefix}: {err}", prefix = self.error_prefix());
 error!("fatal error, panic: {:?}", err);
 panic!("fatal error");
 }
 
 fnfatal_proc_rec(&self,err:&str,proc_res:&ProcRes) -> ! {
-self.error(err);
- proc_res.fatal(None, || ());
+self.fatal_proc_rec_general(err,None, proc_res, || ());
 }
 
-fnfatal_proc_rec_with_ctx(
+/// Underlying implementation of [`Self::fatal_proc_rec`], providing some
+/// extra capabilities not needed by most callers.
+fnfatal_proc_rec_general(
 &self,
 err:&str,
+extra_note:Option<&str>,
 proc_res:&ProcRes,
-on_failure:implFnOnce(Self),
+callback_before_unwind:implFnOnce(),
 ) -> ! {
-self.error(err);
- proc_res.fatal(None, || on_failure(*self));
+println!("\n{prefix}: {err}", prefix = self.error_prefix());
+
+// Some callers want to print additional notes after the main error message.
+ifletSome(note) = extra_note {
+println!("{note}");
+}
+
+// Print the details and output of the subprocess that caused this test to fail.
+println!("{}", proc_res.format_info());
+
+// Some callers want print more context or show a custom diff before the unwind occurs.
+callback_before_unwind();
+
+// Use resume_unwind instead of panic!() to prevent a panic message + backtrace from
+// compiletest, which is unnecessary noise.
+ std::panic::resume_unwind(Box::new(()));
 }
 
 // codegen tests (using FileCheck)
@@ -2080,7 +2101,7 @@ impl<'test> TestCx<'test> {
 ifcfg!(target_os = "freebsd"){"ISO-8859-1"}else{"UTF-8"}
 }
 
-fncompare_to_default_rustdoc(&mutself,out_dir:&Utf8Path){
+fncompare_to_default_rustdoc(&self,out_dir:&Utf8Path){
 if !self.config.has_html_tidy{
 return;
 }
@@ -2957,7 +2978,8 @@ pub struct ProcRes {
 }
 
 implProcRes{
-pubfnprint_info(&self){
+#[must_use]
+pubfnformat_info(&self) -> String{
 fnrender(name:&str,contents:&str) -> String{
 let contents = json::extract_rendered(contents);
 let contents = contents.trim_end();
@@ -2973,24 +2995,13 @@ impl ProcRes {
 }
 }
 
-println!(
+format!(
 "status: {}\ncommand: {}\n{}\n{}\n",
 self.status,
 self.cmdline,
  render("stdout",&self.stdout),
  render("stderr",&self.stderr),
-);
-}
-
-pubfnfatal(&self,err:Option<&str>,on_failure:implFnOnce()) -> ! {
-ifletSome(e) = err {
-println!("\nerror: {}", e);
-}
-self.print_info();
-on_failure();
-// Use resume_unwind instead of panic!() to prevent a panic message + backtrace from
-// compiletest, which is unnecessary noise.
- std::panic::resume_unwind(Box::new(()));
+)
 }
 }
 

src/tools/compiletest/src/runtest/rustdoc.rs

@@ -28,8 +28,8 @@ impl TestCx<'_> {
 }
 let res = self.run_command_to_procres(&mut cmd);
 if !res.status.success(){
-self.fatal_proc_rec_with_ctx("htmldocck failed!",&res, |mut this| {
-this.compare_to_default_rustdoc(&out_dir)
+self.fatal_proc_rec_general("htmldocck failed!",None,&res, || {
+self.compare_to_default_rustdoc(&out_dir);
 });
 }
 }

src/tools/compiletest/src/runtest/rustdoc_json.rs

@@ -29,9 +29,9 @@ impl TestCx<'_> {
 );
 
 if !res.status.success(){
-self.fatal_proc_rec_with_ctx("jsondocck failed!",&res, |_| {
+self.fatal_proc_rec_general("jsondocck failed!",None,&res, || {
 println!("Rustdoc Output:");
- proc_res.print_info();
+println!("{}",proc_res.format_info());
 })
 }