From 409382dd3cd9113757c11d3af2c509dac2edf8f2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 23 Nov 2020 20:50:02 -0500 Subject: [PATCH 1/3] Don't abort rustdoc tests if `tidy` isn't installed Before: ``` Check compiletest suite=rustdoc mode=rustdoc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu) running 396 tests ..................................................2020-11-23T12:12:37.735649Z ERROR compiletest::runtest: fatal error, panic: "failed to run tidy - is it installed? - No such file or directory (os error 2)" F................................................. 100/396 .................................................................................................... 200/396 .................................................................................................... 300/396 ...............................i...............2020-11-23T12:15:00.271271Z ERROR compiletest::runtest: fatal error, panic: "failed to run tidy - is it installed? - No such file or directory (os error 2)" F................................................ ``` After: ``` Check compiletest suite=rustdoc mode=rustdoc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu) running 4 tests .FFF failures: ---- [rustdoc] rustdoc/fn-pointer-arg-name.rs stdout ---- error: htmldocck failed! status: exit code: 1 command: "/usr/bin/python" "/home/joshua/rustc/src/etc/htmldocck.py" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/fn-pointer-arg-name" "/home/joshua/rustc/src/test/rustdoc/fn-pointer-arg-name.rs" stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ 4: @has check failed `XPATH PATTERN` did not match // @has - '//*[@class="rust fn"]' 'pub fn f(callback: fn(len: usize, foo: u32))' Encountered 1 errors ------------------------------------------ info: generating a diff against nightly rustdoc failed to run tidy - is it installed? - Permission denied (os error 13) failed to run tidy - is it installed? - Permission denied (os error 13) # a diff without running `tidy` ``` --- src/tools/compiletest/src/runtest.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ccef005173d92..398f3e31c6ad8 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2394,7 +2394,8 @@ impl<'test> TestCx<'test> { let proc_res = new_rustdoc.document(&compare_dir); if !proc_res.status.success() { - proc_res.fatal(Some("failed to run nightly rustdoc"), || ()); + eprintln!("failed to run nightly rustdoc"); + return; } #[rustfmt::skip] @@ -2409,22 +2410,26 @@ impl<'test> TestCx<'test> { ]; let tidy_dir = |dir| { let tidy = |file: &_| { - Command::new("tidy") - .args(&tidy_args) - .arg(file) - .spawn() - .unwrap_or_else(|err| { - self.fatal(&format!("failed to run tidy - is it installed? - {}", err)) - }) - .wait() - .unwrap() + let tidy_proc = Command::new("tidy").args(&tidy_args).arg(file).spawn(); + match tidy_proc { + Ok(mut proc) => { + proc.wait().unwrap(); + true + } + Err(err) => { + eprintln!("failed to run tidy - is it installed? - {}", err); + false + } + } }; for entry in walkdir::WalkDir::new(dir) { let entry = entry.expect("failed to read file"); if entry.file_type().is_file() && entry.path().extension().and_then(|p| p.to_str()) == Some("html".into()) { - tidy(entry.path()); + if !tidy(entry.path()) { + return; + } } } }; From 804b72af4a478a406e3b73fa690a307c420daf0b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 26 Nov 2020 14:05:54 -0500 Subject: [PATCH 2/3] If tidy isn't installed, only give one error, not many --- src/tools/compiletest/src/common.rs | 3 +++ src/tools/compiletest/src/main.rs | 12 +++++++++++- src/tools/compiletest/src/runtest.rs | 26 ++++++++------------------ 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index eba02333c8cb2..55d25fa7c52c2 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -327,6 +327,9 @@ pub struct Config { /// created in `//rustfix_missing_coverage.txt` pub rustfix_coverage: bool, + /// whether to run `tidy` when a rustdoc test fails + pub has_tidy: bool, + // Configuration for various run-make tests frobbing things like C compilers // or querying about various LLVM component information. pub cc: String, diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 5177dae8a6604..c63bbaf70d3c1 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -14,7 +14,7 @@ use std::ffi::OsString; use std::fs; use std::io::{self, ErrorKind}; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Stdio}; use std::time::SystemTime; use test::ColorConfig; use tracing::*; @@ -43,6 +43,10 @@ fn main() { panic!("Can't find Valgrind to run Valgrind tests"); } + if !config.has_tidy && config.mode == Mode::Rustdoc { + eprintln!("warning: `tidy` is not installed; generated diffs will be harder to read"); + } + log_config(&config); run_tests(config); } @@ -189,6 +193,11 @@ pub fn parse_config(args: Vec) -> Config { let src_base = opt_path(matches, "src-base"); let run_ignored = matches.opt_present("ignored"); + let has_tidy = Command::new("tidy") + .arg("--version") + .stdout(Stdio::null()) + .status() + .map_or(false, |status| status.success()); Config { bless: matches.opt_present("bless"), compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")), @@ -244,6 +253,7 @@ pub fn parse_config(args: Vec) -> Config { remote_test_client: matches.opt_str("remote-test-client").map(PathBuf::from), compare_mode: matches.opt_str("compare-mode").map(CompareMode::parse), rustfix_coverage: matches.opt_present("rustfix-coverage"), + has_tidy, cc: matches.opt_str("cc").unwrap(), cxx: matches.opt_str("cxx").unwrap(), diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 398f3e31c6ad8..d43e75248eb48 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2409,32 +2409,22 @@ impl<'test> TestCx<'test> { "-modify", ]; let tidy_dir = |dir| { - let tidy = |file: &_| { - let tidy_proc = Command::new("tidy").args(&tidy_args).arg(file).spawn(); - match tidy_proc { - Ok(mut proc) => { - proc.wait().unwrap(); - true - } - Err(err) => { - eprintln!("failed to run tidy - is it installed? - {}", err); - false - } - } - }; for entry in walkdir::WalkDir::new(dir) { let entry = entry.expect("failed to read file"); if entry.file_type().is_file() && entry.path().extension().and_then(|p| p.to_str()) == Some("html".into()) { - if !tidy(entry.path()) { - return; - } + let status = + Command::new("tidy").args(&tidy_args).arg(entry.path()).status().unwrap(); + // `tidy` returns 1 if it modified the file. + assert!(status.success() || status.code() == Some(1)); } } }; - tidy_dir(out_dir); - tidy_dir(&compare_dir); + if self.config.has_tidy { + tidy_dir(out_dir); + tidy_dir(&compare_dir); + } let pager = { let output = Command::new("git").args(&["config", "--get", "core.pager"]).output().ok(); From f9b97a8e458b73b7e670a940877f91f9ce0b9ce3 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 26 Nov 2020 15:48:21 -0500 Subject: [PATCH 3/3] Ignore .css files in the diff These are always static and never autogenerated, so the diffs aren't useful. --- src/tools/compiletest/src/runtest.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index d43e75248eb48..7af0d91271b0c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2437,7 +2437,8 @@ impl<'test> TestCx<'test> { }) }; let mut diff = Command::new("diff"); - diff.args(&["-u", "-r"]).args(&[&compare_dir, out_dir]); + // diff recursively, showing context, and excluding .css files + diff.args(&["-u", "-r", "-x", "*.css"]).args(&[&compare_dir, out_dir]); let output = if let Some(pager) = pager { let diff_pid = diff.stdout(Stdio::piped()).spawn().expect("failed to run `diff`");