From a961be9f1338d215a356fca01ffd4db1de56279c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Mon, 7 Oct 2024 07:51:06 +0000 Subject: [PATCH] Remove valgrind test suite and support from compiletest --- src/tools/compiletest/src/common.rs | 8 ---- src/tools/compiletest/src/lib.rs | 6 +-- src/tools/compiletest/src/main.rs | 4 -- src/tools/compiletest/src/runtest.rs | 38 ++----------------- .../compiletest/src/runtest/incremental.rs | 34 ++++++++++++----- src/tools/compiletest/src/runtest/valgrind.rs | 34 ----------------- 6 files changed, 29 insertions(+), 95 deletions(-) delete mode 100644 src/tools/compiletest/src/runtest/valgrind.rs diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index adc89cad72f..17ec6ea4301 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -53,7 +53,6 @@ fn from_str(s: &str) -> Result { string_enum! { #[derive(Clone, Copy, PartialEq, Debug)] pub enum Mode { - RunPassValgrind => "run-pass-valgrind", Pretty => "pretty", DebugInfo => "debuginfo", Codegen => "codegen", @@ -207,13 +206,6 @@ pub struct Config { /// Path to LLVM's bin directory. pub llvm_bin_dir: Option, - /// The valgrind path. - pub valgrind_path: Option, - - /// Whether to fail if we can't run run-pass-valgrind tests under valgrind - /// (or, alternatively, to silently run them like regular run-pass tests). - pub force_valgrind: bool, - /// The path to the Clang executable to run Clang-based tests with. If /// `None` then these tests will be ignored. pub run_clang_based_tests_with: Option, diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index a8355ee9590..d9f64cddf5d 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -53,8 +53,6 @@ pub fn parse_config(args: Vec) -> Config { .reqopt("", "python", "path to python to use for doc tests", "PATH") .optopt("", "jsondocck-path", "path to jsondocck to use for doc tests", "PATH") .optopt("", "jsondoclint-path", "path to jsondoclint to use for doc tests", "PATH") - .optopt("", "valgrind-path", "path to Valgrind executable for Valgrind tests", "PROGRAM") - .optflag("", "force-valgrind", "fail if Valgrind tests cannot be run under Valgrind") .optopt("", "run-clang-based-tests-with", "path to Clang executable", "PATH") .optopt("", "llvm-filecheck", "path to LLVM's FileCheck binary", "DIR") .reqopt("", "src-base", "directory to scan for test files", "PATH") @@ -65,7 +63,7 @@ pub fn parse_config(args: Vec) -> Config { "", "mode", "which sort of compile tests to run", - "run-pass-valgrind | pretty | debug-info | codegen | rustdoc \ + "pretty | debug-info | codegen | rustdoc \ | rustdoc-json | codegen-units | incremental | run-make | ui \ | js-doc-test | mir-opt | assembly | crashes", ) @@ -269,8 +267,6 @@ fn make_absolute(path: PathBuf) -> PathBuf { python: matches.opt_str("python").unwrap(), jsondocck_path: matches.opt_str("jsondocck-path"), jsondoclint_path: matches.opt_str("jsondoclint-path"), - valgrind_path: matches.opt_str("valgrind-path"), - force_valgrind: matches.opt_present("force-valgrind"), run_clang_based_tests_with: matches.opt_str("run-clang-based-tests-with"), llvm_filecheck: matches.opt_str("llvm-filecheck").map(PathBuf::from), llvm_bin_dir: matches.opt_str("llvm-bin-dir").map(PathBuf::from), diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 7b85e6f80b3..9f3eef3776d 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -18,10 +18,6 @@ fn main() { let config = Arc::new(parse_config(env::args().collect())); - if config.valgrind_path.is_none() && config.force_valgrind { - panic!("Can't find Valgrind to run Valgrind tests"); - } - if !config.has_tidy && config.mode == Mode::Rustdoc { eprintln!("warning: `tidy` is not installed; diffs will not be generated"); } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 74d86d2b521..ffde06fdd8b 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -20,9 +20,9 @@ use crate::common::{ Assembly, Codegen, CodegenUnits, CompareMode, Config, CoverageMap, CoverageRun, Crashes, DebugInfo, Debugger, FailMode, Incremental, JsDocTest, MirOpt, PassMode, Pretty, RunMake, - RunPassValgrind, Rustdoc, RustdocJson, TestPaths, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, - UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, - incremental_dir, output_base_dir, output_base_name, output_testname_unique, + Rustdoc, RustdocJson, TestPaths, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, + UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir, + output_base_dir, output_base_name, output_testname_unique, }; use crate::compute_diff::{write_diff, write_filtered_diff}; use crate::errors::{self, Error, ErrorKind}; @@ -49,7 +49,6 @@ mod rustdoc; mod rustdoc_json; mod ui; -mod valgrind; // tidy-alphabet-end #[cfg(test)] @@ -253,7 +252,6 @@ fn run_revision(&self) { self.fatal("cannot use should-ice in a test that is not cfail"); } match self.config.mode { - RunPassValgrind => self.run_valgrind_test(), Pretty => self.run_pretty_test(), DebugInfo => self.run_debuginfo_test(), Codegen => self.run_codegen_test(), @@ -1500,8 +1498,7 @@ fn make_compile_args( Crashes => { set_mir_dump_dir(&mut rustc); } - RunPassValgrind | Pretty | DebugInfo | Rustdoc | RustdocJson | RunMake - | CodegenUnits | JsDocTest => { + Pretty | DebugInfo | Rustdoc | RustdocJson | RunMake | CodegenUnits | JsDocTest => { // do not use JSON output } } @@ -2651,33 +2648,6 @@ fn init_incremental_test(&self) { } } - // FIXME(jieyouxu): `run_rpass_test` is hoisted out here and not in incremental because - // apparently valgrind test falls back to `run_rpass_test` if valgrind isn't available, which - // seems highly questionable to me. - fn run_rpass_test(&self) { - let emit_metadata = self.should_emit_metadata(self.pass_mode()); - let should_run = self.run_if_enabled(); - let proc_res = self.compile_test(should_run, emit_metadata); - - if !proc_res.status.success() { - self.fatal_proc_rec("compilation failed!", &proc_res); - } - - // FIXME(#41968): Move this check to tidy? - if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() { - self.fatal("run-pass tests with expected warnings should be moved to ui/"); - } - - if let WillExecute::Disabled = should_run { - return; - } - - let proc_res = self.exec_compiled_test(); - if !proc_res.status.success() { - self.fatal_proc_rec("test run failed!", &proc_res); - } - } - fn aggressive_rm_rf(&self, path: &Path) -> io::Result<()> { for e in path.read_dir()? { let entry = e?; diff --git a/src/tools/compiletest/src/runtest/incremental.rs b/src/tools/compiletest/src/runtest/incremental.rs index 81b006292e4..bf2b71fef43 100644 --- a/src/tools/compiletest/src/runtest/incremental.rs +++ b/src/tools/compiletest/src/runtest/incremental.rs @@ -1,10 +1,6 @@ use super::{TestCx, WillExecute}; use crate::errors; -// FIXME(jieyouxu): `run_rpass_test` got hoisted out of this because apparently valgrind falls back -// to `run_rpass_test` if valgrind isn't available, which is questionable, but keeping it for -// refactoring changes to preserve current behavior. - impl TestCx<'_> { pub(super) fn run_incremental_test(&self) { // Basic plan for a test incremental/foo/bar.rs: @@ -73,6 +69,30 @@ fn run_cpass_test(&self) { } } + fn run_rpass_test(&self) { + let emit_metadata = self.should_emit_metadata(self.pass_mode()); + let should_run = self.run_if_enabled(); + let proc_res = self.compile_test(should_run, emit_metadata); + + if !proc_res.status.success() { + self.fatal_proc_rec("compilation failed!", &proc_res); + } + + // FIXME(#41968): Move this check to tidy? + if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() { + self.fatal("run-pass tests with expected warnings should be moved to ui/"); + } + + if let WillExecute::Disabled = should_run { + return; + } + + let proc_res = self.exec_compiled_test(); + if !proc_res.status.success() { + self.fatal_proc_rec("test run failed!", &proc_res); + } + } + fn run_cfail_test(&self) { let pm = self.pass_mode(); let proc_res = self.compile_test(WillExecute::No, self.should_emit_metadata(pm)); @@ -115,12 +135,6 @@ fn run_rfail_test(&self) { let proc_res = self.exec_compiled_test(); - // The value our Makefile configures valgrind to return on failure - const VALGRIND_ERR: i32 = 100; - if proc_res.status.code() == Some(VALGRIND_ERR) { - self.fatal_proc_rec("run-fail test isn't valgrind-clean!", &proc_res); - } - let output_to_check = self.get_output(&proc_res); self.check_correct_failure_status(&proc_res); self.check_all_error_patterns(&output_to_check, &proc_res, pm); diff --git a/src/tools/compiletest/src/runtest/valgrind.rs b/src/tools/compiletest/src/runtest/valgrind.rs deleted file mode 100644 index 8d72c4be9ff..00000000000 --- a/src/tools/compiletest/src/runtest/valgrind.rs +++ /dev/null @@ -1,34 +0,0 @@ -use super::{Emit, TestCx, WillExecute}; - -impl TestCx<'_> { - pub(super) fn run_valgrind_test(&self) { - assert!(self.revision.is_none(), "revisions not relevant here"); - - // FIXME(jieyouxu): does this really make any sense? If a valgrind test isn't testing - // valgrind, what is it even testing? - if self.config.valgrind_path.is_none() { - assert!(!self.config.force_valgrind); - return self.run_rpass_test(); - } - - let should_run = self.run_if_enabled(); - let mut proc_res = self.compile_test(should_run, Emit::None); - - if !proc_res.status.success() { - self.fatal_proc_rec("compilation failed!", &proc_res); - } - - if let WillExecute::Disabled = should_run { - return; - } - - let mut new_config = self.config.clone(); - new_config.runner = new_config.valgrind_path.clone(); - let new_cx = TestCx { config: &new_config, ..*self }; - proc_res = new_cx.exec_compiled_test(); - - if !proc_res.status.success() { - self.fatal_proc_rec("test run failed!", &proc_res); - } - } -}