From 051f9ec694ec8c173b551d674cd17b8b989a9ee8 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Sat, 24 Apr 2021 01:20:48 +0000 Subject: [PATCH 1/6] Add --run flag to compiletest This controls whether run-* tests actually get run. --- src/tools/compiletest/src/common.rs | 3 +++ src/tools/compiletest/src/main.rs | 7 +++++++ src/tools/compiletest/src/runtest.rs | 21 ++++++++++++++++----- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index b7693a3cb1431..9a2166675d8ab 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -249,6 +249,9 @@ pub struct Config { /// Force the pass mode of a check/build/run-pass test to this mode. pub force_pass_mode: Option, + /// Explicitly enable or disable running. + pub run: Option, + /// Write out a parseable log of tests that were run pub logfile: Option, diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 480916018619d..5bf1b55e45b4a 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -87,6 +87,7 @@ pub fn parse_config(args: Vec) -> Config { "force {check,build,run}-pass tests to this mode.", "check | build | run", ) + .optopt("", "run", "whether to execute run-* tests", "auto | always | never") .optflag("", "ignored", "run tests marked as ignored") .optflag("", "exact", "filters match exactly") .optopt( @@ -234,6 +235,12 @@ pub fn parse_config(args: Vec) -> Config { mode.parse::() .unwrap_or_else(|_| panic!("unknown `--pass` option `{}` given", mode)) }), + run: matches.opt_str("run").and_then(|mode| match mode.as_str() { + "auto" => None, + "always" => Some(true), + "never" => Some(false), + _ => panic!("unknown `--run` option `{}` given", mode), + }), logfile: matches.opt_str("logfile").map(|s| PathBuf::from(&s)), runtool: matches.opt_str("runtool"), host_rustcflags: matches.opt_str("host-rustcflags"), diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ecbaccf744dcd..c758b977573ae 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -317,6 +317,7 @@ enum TestOutput { enum WillExecute { Yes, No, + Disabled, } /// Should `--emit metadata` be used? @@ -357,13 +358,22 @@ impl<'test> TestCx<'test> { } fn should_run(&self, pm: Option) -> WillExecute { - match self.config.mode { + let test_should_run = match self.config.mode { Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => { - WillExecute::Yes + true } - MirOpt if pm == Some(PassMode::Run) => WillExecute::Yes, - Ui | MirOpt => WillExecute::No, + MirOpt if pm == Some(PassMode::Run) => true, + Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), + }; + let enabled = self.config.run.unwrap_or_else(|| { + // Auto-detect whether to run based on the platform. + !self.config.target.ends_with("-fuchsia") + }); + match (test_should_run, enabled) { + (false, _) => WillExecute::No, + (true, true) => WillExecute::Yes, + (true, false) => WillExecute::Disabled, } } @@ -1531,7 +1541,8 @@ impl<'test> TestCx<'test> { // Only use `make_exe_name` when the test ends up being executed. let output_file = match will_execute { WillExecute::Yes => TargetLocation::ThisFile(self.make_exe_name()), - WillExecute::No => TargetLocation::ThisDirectory(self.output_base_dir()), + WillExecute::No | WillExecute::Disabled => + TargetLocation::ThisDirectory(self.output_base_dir()), }; let allow_unused = match self.config.mode { From 09783815b29fe6a8d0299bf883dba733f8a6fd1d Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 28 Apr 2021 23:50:16 +0000 Subject: [PATCH 2/6] Add run flag to bootstrap test --- src/bootstrap/builder/tests.rs | 3 +++ src/bootstrap/flags.rs | 15 +++++++++++++++ src/bootstrap/test.rs | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index a881512e988ed..4d7c207e3ab8b 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -489,6 +489,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; let build = Build::new(config); @@ -529,6 +530,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; let build = Build::new(config); @@ -584,6 +586,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; // Make sure rustfmt binary not being found isn't an error. config.channel = "beta".to_string(); diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 6044899c237e2..13d11cded303a 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -103,6 +103,7 @@ pub enum Subcommand { bless: bool, compare_mode: Option, pass: Option, + run: Option, test_args: Vec, rustc_args: Vec, fail_fast: bool, @@ -293,6 +294,12 @@ To learn more about a subcommand, run `./x.py -h`", "force {check,build,run}-pass tests to this mode.", "check | build | run", ); + opts.optopt( + "", + "run", + "whether to execute run-* tests", + "auto | always | never", + ); opts.optflag( "", "rustfix-coverage", @@ -556,6 +563,7 @@ Arguments: bless: matches.opt_present("bless"), compare_mode: matches.opt_str("compare-mode"), pass: matches.opt_str("pass"), + run: matches.opt_str("run"), test_args: matches.opt_strs("test-args"), rustc_args: matches.opt_strs("rustc-args"), fail_fast: !matches.opt_present("no-fail-fast"), @@ -742,6 +750,13 @@ impl Subcommand { } } + pub fn run(&self) -> Option<&str> { + match *self { + Subcommand::Test { ref run, .. } => run.as_ref().map(|s| &s[..]), + _ => None, + } + } + pub fn open(&self) -> bool { match *self { Subcommand::Doc { open, .. } => open, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index b9d7ecf8c0eb1..1eccfe102be8f 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1207,6 +1207,11 @@ note: if you're sure you want to do this, please open an issue as to why. In the cmd.arg(pass); } + if let Some(ref run) = builder.config.cmd.run() { + cmd.arg("--run"); + cmd.arg(run); + } + if let Some(ref nodejs) = builder.config.nodejs { cmd.arg("--nodejs").arg(nodejs); } From 0b2e908691a2d5d35ebd877a2c3339b230b81eb0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 00:13:56 +0000 Subject: [PATCH 3/6] Add support for --run for non-ui tests --- src/bootstrap/flags.rs | 7 +--- src/tools/compiletest/src/runtest.rs | 58 ++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 13d11cded303a..dd1e48a14684f 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -294,12 +294,7 @@ To learn more about a subcommand, run `./x.py -h`", "force {check,build,run}-pass tests to this mode.", "check | build | run", ); - opts.optopt( - "", - "run", - "whether to execute run-* tests", - "auto | always | never", - ); + opts.optopt("", "run", "whether to execute run-* tests", "auto | always | never"); opts.optflag( "", "rustfix-coverage", diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c758b977573ae..a044c4425b4e6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -359,22 +359,20 @@ impl<'test> TestCx<'test> { fn should_run(&self, pm: Option) -> WillExecute { let test_should_run = match self.config.mode { - Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => { - true - } + Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => true, MirOpt if pm == Some(PassMode::Run) => true, Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), }; + if test_should_run { self.run_if_enabled() } else { WillExecute::No } + } + + fn run_if_enabled(&self) -> WillExecute { let enabled = self.config.run.unwrap_or_else(|| { // Auto-detect whether to run based on the platform. !self.config.target.ends_with("-fuchsia") }); - match (test_should_run, enabled) { - (false, _) => WillExecute::No, - (true, true) => WillExecute::Yes, - (true, false) => WillExecute::Disabled, - } + if enabled { WillExecute::Yes } else { WillExecute::Disabled } } fn should_run_successfully(&self, pm: Option) -> bool { @@ -449,12 +447,17 @@ impl<'test> TestCx<'test> { fn run_rfail_test(&self) { let pm = self.pass_mode(); - let proc_res = self.compile_test(WillExecute::Yes, self.should_emit_metadata(pm)); + let should_run = self.run_if_enabled(); + let proc_res = self.compile_test(should_run, self.should_emit_metadata(pm)); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); } + if let WillExecute::Disabled = should_run { + return; + } + let proc_res = self.exec_compiled_test(); // The value our Makefile configures valgrind to return on failure @@ -493,12 +496,17 @@ impl<'test> TestCx<'test> { fn run_rpass_test(&self) { let emit_metadata = self.should_emit_metadata(self.pass_mode()); - let proc_res = self.compile_test(WillExecute::Yes, emit_metadata); + 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); } + if let WillExecute::Disabled = should_run { + return; + } + // FIXME(#41968): Move this check to tidy? let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); assert!( @@ -520,12 +528,17 @@ impl<'test> TestCx<'test> { return self.run_rpass_test(); } - let mut proc_res = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let mut proc_res = self.compile_test(should_run, EmitMetadata::No); 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.runtool = new_config.valgrind_path.clone(); let new_cx = TestCx { config: &new_config, ..*self }; @@ -742,10 +755,14 @@ impl<'test> TestCx<'test> { fn run_debuginfo_cdb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) - let compile_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compile_result = self.compile_test(should_run, EmitMetadata::No); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -836,10 +853,14 @@ impl<'test> TestCx<'test> { let mut cmds = commands.join("\n"); // compile test file (it should have 'compile-flags:-g' in the header) - let compiler_run_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compiler_run_result = self.compile_test(should_run, EmitMetadata::No); if !compiler_run_result.status.success() { self.fatal_proc_rec("compilation failed!", &compiler_run_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -1054,10 +1075,14 @@ impl<'test> TestCx<'test> { fn run_debuginfo_lldb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) - let compile_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compile_result = self.compile_test(should_run, EmitMetadata::No); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -1541,8 +1566,9 @@ impl<'test> TestCx<'test> { // Only use `make_exe_name` when the test ends up being executed. let output_file = match will_execute { WillExecute::Yes => TargetLocation::ThisFile(self.make_exe_name()), - WillExecute::No | WillExecute::Disabled => - TargetLocation::ThisDirectory(self.output_base_dir()), + WillExecute::No | WillExecute::Disabled => { + TargetLocation::ThisDirectory(self.output_base_dir()) + } }; let allow_unused = match self.config.mode { From e282fd045a96793fb060c224887b9807370bd9d1 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 00:20:21 +0000 Subject: [PATCH 4/6] Include --run in stamp hash --- src/tools/compiletest/src/runtest.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a044c4425b4e6..c87a0c8c8d9ec 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -259,6 +259,7 @@ pub fn run(config: Config, testpaths: &TestPaths, revision: Option<&str>) { pub fn compute_stamp_hash(config: &Config) -> String { let mut hash = DefaultHasher::new(); config.stage_id.hash(&mut hash); + config.run.hash(&mut hash); match config.debugger { Some(Debugger::Cdb) => { From f64c45a7d21f69251b05d796ba52a04e6201eefd Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 01:02:07 +0000 Subject: [PATCH 5/6] Add needs-run-enabled directive for should-fail tests I was wary of doing any automatic disabling here, since should-fail is how we test compiletest itself. --- src/test/debuginfo/should-fail.rs | 1 + src/test/ui/meta/revision-bad.rs | 1 + src/tools/compiletest/src/common.rs | 9 +++++++++ src/tools/compiletest/src/header.rs | 4 ++++ src/tools/compiletest/src/runtest.rs | 6 +----- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/test/debuginfo/should-fail.rs b/src/test/debuginfo/should-fail.rs index 1e0d22cbce404..eef6d99d2a91c 100644 --- a/src/test/debuginfo/should-fail.rs +++ b/src/test/debuginfo/should-fail.rs @@ -2,6 +2,7 @@ // == Test [gdb|lldb]-[command|check] are parsed correctly === // should-fail +// needs-run-enabled // compile-flags:-g // === GDB TESTS =================================================================================== diff --git a/src/test/ui/meta/revision-bad.rs b/src/test/ui/meta/revision-bad.rs index 01f1518c1c6ed..37ddbe99a9f03 100644 --- a/src/test/ui/meta/revision-bad.rs +++ b/src/test/ui/meta/revision-bad.rs @@ -4,6 +4,7 @@ // run-fail // revisions: foo bar // should-fail +// needs-run-enabled //[foo] error-pattern:bar //[bar] error-pattern:foo diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 9a2166675d8ab..2a14d8a39990a 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -351,6 +351,15 @@ pub struct Config { pub npm: Option, } +impl Config { + pub fn run_enabled(&self) -> bool { + self.run.unwrap_or_else(|| { + // Auto-detect whether to run based on the platform. + !self.target.ends_with("-fuchsia") + }) + } +} + #[derive(Debug, Clone)] pub struct TestPaths { pub file: PathBuf, // e.g., compile-test/foo/bar/baz.rs diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index f31a24738df6c..56527420c0d08 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -85,6 +85,10 @@ impl EarlyProps { props.ignore = true; } + if !config.run_enabled() && config.parse_name_directive(ln, "needs-run-enabled") { + props.ignore = true; + } + if !rustc_has_sanitizer_support && config.parse_name_directive(ln, "needs-sanitizer-support") { diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c87a0c8c8d9ec..c606aa1dfbfd4 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -369,11 +369,7 @@ impl<'test> TestCx<'test> { } fn run_if_enabled(&self) -> WillExecute { - let enabled = self.config.run.unwrap_or_else(|| { - // Auto-detect whether to run based on the platform. - !self.config.target.ends_with("-fuchsia") - }); - if enabled { WillExecute::Yes } else { WillExecute::Disabled } + if self.config.run_enabled() { WillExecute::Yes } else { WillExecute::Disabled } } fn should_run_successfully(&self, pm: Option) -> bool { From 1e46b18fec554664f35c73079bec0964429b9fa8 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 28 Apr 2021 23:54:39 +0000 Subject: [PATCH 6/6] Fix help for profile flags --- src/bootstrap/flags.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index dd1e48a14684f..d961e067db37c 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -223,8 +223,8 @@ To learn more about a subcommand, run `./x.py -h`", VALUE overrides the skip-rebuild option in config.toml.", "VALUE", ); - opts.optopt("", "rust-profile-generate", "rustc error format", "FORMAT"); - opts.optopt("", "rust-profile-use", "rustc error format", "FORMAT"); + opts.optopt("", "rust-profile-generate", "generate PGO profile with rustc build", "FORMAT"); + opts.optopt("", "rust-profile-use", "use PGO profile for rustc build", "FORMAT"); // We can't use getopt to parse the options until we have completed specifying which // options are valid, but under the current implementation, some options are conditional on