libtest: do not early exit from test runners

compiler/rustc_builtin_macros/src/test_harness.rs

@@ -110,8 +110,7 @@ impl TestHarnessGenerator<'_> {
                 Some(node_id),
             );
             for test in &mut tests {
-                // See the comment on `mk_main` for why we're using
-                // `apply_mark` directly.
+                // See the comment on `add_main` for why we're using `apply_mark` directly.
                 test.ident.span =
                     test.ident.span.apply_mark(expn_id.to_expn_id(), Transparency::Opaque);
             }
@@ -127,7 +126,7 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> {
         self.add_test_cases(ast::CRATE_NODE_ID, c.spans.inner_span, prev_tests);
 
         // Create a main function to run our tests
-        c.items.push(mk_main(&mut self.cx));
+        add_main(&mut self.cx, c);
     }
 
     fn visit_item(&mut self, item: &mut ast::Item) {
@@ -288,16 +287,20 @@ fn generate_test_harness(
 /// [`TestCtxt::reexport_test_harness_main`] provides a different name for the `main`
 /// function and [`TestCtxt::test_runner`] provides a path that replaces
 /// `test::test_main_env_args`.
-fn mk_main(cx: &mut TestCtxt<'_>) -> Box<ast::Item> {
+fn add_main(cx: &mut TestCtxt<'_>, c: &mut ast::Crate) {
     let sp = cx.def_site;
     let ecx = &cx.ext_cx;
+    // `sp` has def-site hygiene so should not clash with user-defined names.
     let test_ident = Ident::new(sym::test, sp);
 
-    let runner_name =
-        if cx.panic_strategy.unwinds() { "test_main_env_args" } else { "test_main_env_args_abort" };
-
     // test::test_main_env_args(...)
     let mut test_runner = cx.test_runner.clone().unwrap_or_else(|| {
+        // Built-in runner name depends on panic strategy.
+        let runner_name = if cx.panic_strategy.unwinds() {
+            "test_main_env_args"
+        } else {
+            "test_main_env_args_abort"
+        };
         ecx.path(sp, vec![test_ident, Ident::from_str_and_span(runner_name, sp)])
     });
 
@@ -308,10 +311,8 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> Box<ast::Item> {
     let call_test_main = ecx.stmt_expr(call_test_main);
 
     // extern crate test
-    let test_extern_stmt = ecx.stmt_item(
-        sp,
-        ecx.item(sp, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None, test_ident)),
-    );
+    let test_extern_stmt =
+        ecx.item(sp, ast::AttrVec::new(), ast::ItemKind::ExternCrate(None, test_ident));
 
     // #[rustc_main]
     let main_attr = ecx.attr_word(sym::rustc_main, sp);
@@ -320,20 +321,18 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> Box<ast::Item> {
     // #[doc(hidden)]
     let doc_hidden_attr = ecx.attr_nested_word(sym::doc, sym::hidden, sp);
 
-    // pub fn main() { ... }
-    // FIXME: it would be nice if we could use `std::process::ExitCode` as return type here, and
-    // remove all early-exit from libtest itself. Or rather, it should be `test::ExitCode` so we
-    // don't depend on whatever `std` may be. This needs the `extern crate test` to be *outside*
-    // `main`. But naively moving it out causes ICEs that give no hint as to what is wrong.
-    let main_ret_ty = ecx.ty(sp, ast::TyKind::Tup(ThinVec::new()));
-
-    // If no test runner is provided we need to import the test crate
-    let main_body = if cx.test_runner.is_none() {
-        ecx.block(sp, thin_vec![test_extern_stmt, call_test_main])
+    // pub fn main() -> ExitCode { ... }
+    let main_ret_ty = if cx.test_runner.is_none() {
+        // Built-in runner has return type `ExitCode`.
+        let exit_code_path = vec![test_ident, Ident::from_str_and_span("ExitCode", sp)];
+        ecx.ty(sp, ast::TyKind::Path(None, ecx.path(sp, exit_code_path)))
     } else {
-        ecx.block(sp, thin_vec![call_test_main])
+        // User-defined runners have return type `()`.
+        ecx.ty(sp, ast::TyKind::Tup(ThinVec::new()))
     };
 
+    let main_body = ecx.block(sp, thin_vec![call_test_main]);
+
     let decl = ecx.fn_decl(ThinVec::new(), ast::FnRetTy::Ty(main_ret_ty));
     let sig = ast::FnSig { decl, header: ast::FnHeader::default(), span: sp };
     let defaultness = ast::Defaultness::Implicit;
@@ -365,8 +364,8 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> Box<ast::Item> {
     });
 
     // Integrate the new item into existing module structures.
-    let main = AstFragment::Items(smallvec![main]);
-    cx.ext_cx.monotonic_expander().fully_expand_fragment(main).make_items().pop().unwrap()
+    let items = AstFragment::Items(smallvec![test_extern_stmt, main]);
+    c.items.extend(cx.ext_cx.monotonic_expander().fully_expand_fragment(items).make_items());
 }
 
 /// Creates a slice containing every test like so:

library/test/src/lib.rs

@@ -18,7 +18,6 @@
 #![doc(test(attr(deny(warnings))))]
 #![doc(rust_logo)]
 #![feature(rustdoc_internals)]
-#![feature(exitcode_exit_method)]
 #![feature(file_buffered)]
 #![feature(internal_output_capture)]
 #![feature(io_const_error)]
@@ -31,35 +30,31 @@
 #![warn(rustdoc::unescaped_backticks)]
 #![warn(unreachable_pub)]
 
+pub use std::process::ExitCode; // used by rustc-generated test harness
+
 pub use cli::TestOpts;
 
 pub use self::ColorConfig::*;
 pub use self::bench::{Bencher, black_box};
 pub use self::console::run_tests_console;
 pub use self::options::{ColorConfig, Options, OutputFormat, RunIgnored, ShouldPanic};
-pub use self::types::TestName::*;
 pub use self::types::*;
 
-// Module to be used by rustc to compile tests in libtest
+// Make some items publicly available for our own tests.
 pub mod test {
     pub use crate::bench::Bencher;
     pub use crate::cli::{TestOpts, parse_opts};
     pub use crate::helpers::metrics::{Metric, MetricMap};
     pub use crate::options::{Options, RunIgnored, RunStrategy, ShouldPanic};
     pub use crate::test_result::{TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk};
     pub use crate::time::{TestExecTime, TestTimeOptions};
-    pub use crate::types::{
-        DynTestFn, DynTestName, StaticBenchFn, StaticTestFn, StaticTestName, TestDesc,
-        TestDescAndFn, TestId, TestList, TestListOrder, TestName, TestType,
-    };
-    pub use crate::{assert_test_result, filter_tests, run_test, test_main, test_main_env_args};
 }
 
 use std::collections::VecDeque;
 use std::io::prelude::Write;
 use std::mem::ManuallyDrop;
 use std::panic::{self, AssertUnwindSafe, PanicHookInfo, catch_unwind};
-use std::process::{self, Command, ExitCode, Termination};
+use std::process::{self, Command, Termination};
 use std::sync::mpsc::{Sender, channel};
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
@@ -170,34 +165,28 @@ fn test_main_inner(args: &[String], tests: TestList<'_>, options: Option<Options
     ExitCode::SUCCESS
 }
 
-/// A variant that takes the arguments from the command line. Exits the process if there
-/// was an error, returns on success.
+/// A variant that takes the arguments from the command line.
 ///
 /// This is the entry point for the main function generated by `rustc --test`
 /// when panic=unwind.
-pub fn test_main_env_args(tests: &[&TestDescAndFn]) {
+pub fn test_main_env_args(tests: &[&TestDescAndFn]) -> ExitCode {
     // This is supposed to be reasonably fast even in Miri. In particular, when invoked via `--exact
     // test`, we want the entire invocation to be `O(log n)` in the number of tests: never iterate
     // the entire test list (as that list could be big)!
     let args = env::args().collect::<Vec<_>>();
     // Tests are sorted by name at compile time by mk_tests_slice.
     let tests = TestList::new(tests, TestListOrder::Sorted);
-    let exit = test_main_inner(&args, tests, None);
-    // We do *not* want to exit here on success, that breaks coverage tracking on Windows.
-    if exit != std::process::ExitCode::SUCCESS {
-        exit.exit_process();
-    }
+    test_main_inner(&args, tests, None)
 }
 
-/// A variant that takes the arguments from the command line. Exits the process if there
-/// was an error, returns on success.
+/// A variant that takes the arguments from the command line.
 ///
 /// Runs tests in panic=abort mode, which involves spawning subprocesses for
 /// tests. If we are invoked as subprocess, this function does not return.
 ///
 /// This is the entry point for the main function generated by `rustc --test`
 /// when panic=abort.
-pub fn test_main_env_args_abort(tests: &[&TestDescAndFn]) {
+pub fn test_main_env_args_abort(tests: &[&TestDescAndFn]) -> ExitCode {
     // If we're being run in SpawnedSecondary mode, run the test here. run_test
     // will then exit the process.
     if let Ok(name) = env::var(SECONDARY_TEST_INVOKER_VAR) {
@@ -246,10 +235,7 @@ pub fn test_main_env_args_abort(tests: &[&TestDescAndFn]) {
     let args = env::args().collect::<Vec<_>>();
     // Tests are sorted by name at compile time by mk_tests_slice.
     let tests = TestList::new(tests, TestListOrder::Sorted);
-    let exit = test_main_inner(&args, tests, Some(Options::new().panic_abort(true)));
-    if exit != std::process::ExitCode::SUCCESS {
-        exit.exit_process();
-    }
+    test_main_inner(&args, tests, Some(Options::new().panic_abort(true)))
 }
 
 /// Public API used by rustdoc to display the `total` and `compilation` times in the expected

src/tools/clippy/clippy_lints/src/items_after_test_module.rs

@@ -65,9 +65,9 @@ impl LateLintPass<'_> for ItemsAfterTestModule {
 
         let after: Vec<_> = items
             .filter(|item| {
-                // Ignore the generated test main function
-                if let ItemKind::Fn { ident, .. } = item.kind
-                    && ident.name == sym::main
+                // Ignore the generated test main function and `extern crate test`
+                if (matches!(item.kind, ItemKind::Fn { ident, .. } if ident.name == sym::main)
+                    || matches!(item.kind, ItemKind::ExternCrate(None, ident) if ident.name == sym::test))
                     && item.span.ctxt().outer_expn_data().kind == ExpnKind::AstPass(AstPass::TestHarness)
                 {
                     false

tests/pretty/tests-are-sorted.pp

@@ -80,10 +80,10 @@
                 test::assert_test_result(a_test())),
     };
 fn a_test() {}
+extern crate test;
 #[rustc_main]
 #[coverage(off)]
 #[doc(hidden)]
-pub fn main() -> () {
-    extern crate test;
+pub fn main() -> test::ExitCode {
     test::test_main_env_args(&[&a_test, &m_test, &z_test])
 }