diff --git a/.cargo/config b/.cargo/config index f969f9004b269..4beb99d0746a9 100644 --- a/.cargo/config +++ b/.cargo/config @@ -7,3 +7,4 @@ xfix = "run --package x --bin x -- fix" xtest = "run --package x --bin x -- test" xlint = "run --package x --bin x -- lint" xbuild = "run --package x --bin x -- build" +nextest = "run --package x --bin x -- nextest" diff --git a/Cargo.lock b/Cargo.lock index 3e24bce3f5527..81668c47ad0b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,9 +148,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.38" +version = "1.0.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afddf7f520a80dbf76e6f50a35bca42a2331ef227a28b3b6dc5c2e2338d114b1" +checksum = "81cddc5f91628367664cc7c69714ff08deee8a3efc54623011c772544d7b2767" [[package]] name = "arbitrary" @@ -688,9 +688,9 @@ checksum = "4964518bd3b4a8190e832886cdc0da9794f12e8e6c1613a9e90ff331c4c8724b" [[package]] name = "camino" -version = "1.0.3" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fb25ad5c4211e3c7abded8f00b480e6d1608845dcf14157e9e81d5432e9ac16" +checksum = "d4648c6d00a709aa069a236adcaae4f605a6241c72bf5bee79331a4b625921a9" dependencies = [ "serde", ] @@ -2704,6 +2704,18 @@ version = "0.4.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88d7ed2934d741c6b37e33e3832298e8850b53fd2d2bea03873375596c7cea4e" +[[package]] +name = "duct" +version = "0.13.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fc6a0a59ed0888e0041cf708e66357b7ae1a82f1c67247e1f93b5e0818f7d8d" +dependencies = [ + "libc", + "once_cell", + "os_pipe", + "shared_child", +] + [[package]] name = "ed25519" version = "1.0.3" @@ -4992,6 +5004,16 @@ dependencies = [ "num-traits", ] +[[package]] +name = "os_pipe" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb233f06c2307e1f5ce2ecad9f8121cffbbee2c95428f44ea85222e460d0d213" +dependencies = [ + "libc", + "winapi 0.3.9", +] + [[package]] name = "parking_lot" version = "0.11.1" @@ -6511,6 +6533,16 @@ dependencies = [ "opaque-debug 0.3.0", ] +[[package]] +name = "shared_child" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6be9f7d5565b1483af3e72975e2dee33879b3b86bd48c0929fccf6585d79e65a" +dependencies = [ + "libc", + "winapi 0.3.9", +] + [[package]] name = "shell-words" version = "1.0.0" @@ -6536,6 +6568,16 @@ dependencies = [ "thiserror", ] +[[package]] +name = "signal-hook" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6aa894ef3fade0ee7243422f4fbbd6c2b48e6de767e621d37ef65f2310f53cea" +dependencies = [ + "libc", + "signal-hook-registry", +] + [[package]] name = "signal-hook-registry" version = "1.3.0" @@ -7091,6 +7133,27 @@ dependencies = [ "vm", ] +[[package]] +name = "testrunner" +version = "0.1.0" +source = "git+https://github.com/diem/diem-devtools?branch=main#3cc8015b05cf591a5a96801aee394f7df92d946c" +dependencies = [ + "aho-corasick", + "anyhow", + "atty", + "camino", + "crossbeam-channel", + "duct", + "num_cpus", + "once_cell", + "rayon", + "serde", + "serde_json", + "signal-hook", + "structopt 0.3.21", + "termcolor", +] + [[package]] name = "textwrap" version = "0.11.0" @@ -7983,6 +8046,7 @@ version = "0.1.0" dependencies = [ "anyhow", "camino", + "cargo_metadata", "chrono", "colored-diff", "determinator", @@ -7998,6 +8062,7 @@ dependencies = [ "serde", "serde_json", "structopt 0.3.21", + "testrunner", "toml", "x-core", "x-lint", diff --git a/devtools/x-core/src/lib.rs b/devtools/x-core/src/lib.rs index faac226f28ebc..3bea5a0240e3f 100644 --- a/devtools/x-core/src/lib.rs +++ b/devtools/x-core/src/lib.rs @@ -96,6 +96,23 @@ impl XCoreContext { Ok(self.package_graph_plus()?.head()) } + /// For a given list of workspace packages, returns a tuple of (known, unknown) packages. + /// + /// Initializes the package graph if it isn't already done so, and returns an error if the + pub fn partition_workspace_names<'a, B>( + &self, + names: impl IntoIterator, + ) -> Result<(B, B)> + where + B: Default + Extend<&'a str>, + { + let workspace = self.package_graph()?.workspace(); + let (known, unknown) = names + .into_iter() + .partition(|name| workspace.contains_name(name)); + Ok((known, unknown)) + } + /// Returns information about the subsets for this workspace. pub fn subsets(&self) -> Result<&WorkspaceSubsets> { Ok(self.package_graph_plus()?.suffix()) diff --git a/devtools/x/Cargo.toml b/devtools/x/Cargo.toml index c3364459e611a..a9f37880ce6aa 100644 --- a/devtools/x/Cargo.toml +++ b/devtools/x/Cargo.toml @@ -9,6 +9,7 @@ license = "Apache-2.0" [dependencies] camino = "1.0.3" +cargo_metadata = "0.13.1" determinator = "0.4.0" serde = { version = "1.0.124", features = ["derive"] } serde_json = "1.0.64" @@ -25,6 +26,7 @@ chrono = "0.4.19" globset = "0.4.6" regex = "1.4.3" rayon = "1.5.0" +testrunner = { git = "https://github.com/diem/diem-devtools", branch = "main" } indexmap = "1.6.2" x-core = { path = "../x-core" } x-lint = { path = "../x-lint" } diff --git a/devtools/x/src/build.rs b/devtools/x/src/build.rs index 103eb49ca596d..b5e05bab9fe3b 100644 --- a/devtools/x/src/build.rs +++ b/devtools/x/src/build.rs @@ -13,9 +13,6 @@ use structopt::StructOpt; pub struct Args { #[structopt(flatten)] package_args: SelectedPackageArgs, - #[structopt(long, number_of_values = 1)] - /// Package to exclude (see `cargo help pkgid`) - exclude: Vec, #[structopt(flatten)] build_args: BuildArgs, #[structopt(long, parse(from_os_str))] diff --git a/devtools/x/src/cargo.rs b/devtools/x/src/cargo.rs index 20aad2d04dc20..2c715f369d4ce 100644 --- a/devtools/x/src/cargo.rs +++ b/devtools/x/src/cargo.rs @@ -7,12 +7,14 @@ use crate::{ utils::{apply_sccache_if_possible, project_root}, Result, }; -use anyhow::anyhow; +use anyhow::{anyhow, Context}; +use cargo_metadata::Message; use indexmap::map::IndexMap; use log::{info, warn}; use std::{ env, ffi::{OsStr, OsString}, + io::Cursor, path::Path, process::{Command, Output, Stdio}, time::Instant, @@ -207,12 +209,10 @@ impl Cargo { } /// Runs this command, capturing the standard output into a `Vec`. - /// No logging/timing will be displayed as the result of this call from x. - #[allow(dead_code)] + /// Standard error is forwarded. pub fn run_with_output(&mut self) -> Result> { self.inner.stderr(Stdio::inherit()); - // Since system out hijacked don't log for this command - self.do_run(false).map(|o| o.stdout) + self.do_run(true).map(|o| o.stdout) } /// Internal run command, where the magic happens. @@ -332,14 +332,39 @@ impl<'a> CargoCommand<'a> { return Ok(()); } + let mut cargo = self.prepare_cargo(packages); + cargo.run() + } + + /// Runs this command on the selected packages, returning the standard output as a bytestring. + pub fn run_capture_messages( + &self, + packages: &SelectedPackages<'_>, + ) -> Result>> { + // Early return if we have no packages to run. + let output = if !packages.should_invoke() { + info!("no packages to {}: exiting early", self.as_str()); + vec![] + } else { + let mut cargo = self.prepare_cargo(packages); + cargo.args(&["--message-format", "json"]); + cargo.run_with_output()? + }; + + Ok(Message::parse_stream(Cursor::new(output)) + .map(|message| message.context("error while parsing message from Cargo"))) + } + + fn prepare_cargo(&self, packages: &SelectedPackages<'_>) -> Cargo { let mut cargo = Cargo::new(self.cargo_config(), self.as_str(), true); cargo .current_dir(project_root()) .args(self.direct_args()) .packages(packages) .pass_through(self.pass_through_args()) - .envs(self.get_extra_env().to_owned()) - .run() + .envs(self.get_extra_env().to_owned()); + + cargo } pub fn as_str(&self) -> &'static str { diff --git a/devtools/x/src/cargo/selected_package.rs b/devtools/x/src/cargo/selected_package.rs index 0f24fd07868e0..f9619b4324843 100644 --- a/devtools/x/src/cargo/selected_package.rs +++ b/devtools/x/src/cargo/selected_package.rs @@ -4,6 +4,7 @@ use crate::{changed_since::changed_since_impl, context::XContext, Result}; use anyhow::anyhow; use guppy::graph::DependencyDirection; +use log::warn; use std::collections::BTreeSet; use structopt::StructOpt; use x_core::WorkspaceStatus; @@ -17,6 +18,9 @@ pub struct SelectedPackageArgs { #[structopt(long, short, number_of_values = 1)] /// Run on the specified members (package subsets) pub(crate) members: Vec, + #[structopt(long, number_of_values = 1)] + /// Exclude packages + pub(crate) exclude: Vec, #[structopt(long, short)] /// Run on packages changed since the merge base of this commit changed_since: Option, @@ -70,7 +74,26 @@ impl SelectedPackageArgs { ); } - Ok(SelectedPackages::new(includes)) + let mut ret = SelectedPackages::new(includes); + + if !self.exclude.is_empty() { + let workspace = xctx.core().package_graph()?.workspace(); + // Check that all the excluded package names are valid. + let (known, unknown): (Vec<_>, Vec<_>) = xctx + .core() + .partition_workspace_names(self.exclude.iter().map(|package| package.as_str()))?; + if !unknown.is_empty() { + warn!( + "excluded package(s) `{}` not found in workspace `{}`", + unknown.join(", "), + workspace.root() + ) + } + + ret.add_excludes(known); + } + + Ok(ret) } } diff --git a/devtools/x/src/main.rs b/devtools/x/src/main.rs index 97c9212fd1bfd..c8b41e0e2a14c 100644 --- a/devtools/x/src/main.rs +++ b/devtools/x/src/main.rs @@ -24,6 +24,7 @@ mod generate_summaries; mod generate_workspace_hack; mod installer; mod lint; +mod nextest; mod playground; mod test; mod tools; @@ -68,6 +69,9 @@ enum Command { #[structopt(name = "test")] /// Run tests Test(test::Args), + #[structopt(name = "nextest")] + /// Run tests with new test runner + Nextest(nextest::Args), #[structopt(name = "tools")] /// Run tests Tools(tools::Args), @@ -115,6 +119,7 @@ fn main() -> Result<()> { match args.cmd { Command::Tools(args) => tools::run(args, xctx), Command::Test(args) => test::run(args, xctx), + Command::Nextest(args) => nextest::run(args, xctx), Command::Build(args) => build::run(args, xctx), Command::ChangedSince(args) => changed_since::run(args, xctx), Command::Check(args) => check::run(args, xctx), diff --git a/devtools/x/src/nextest.rs b/devtools/x/src/nextest.rs new file mode 100644 index 0000000000000..f8b454bf4b6f2 --- /dev/null +++ b/devtools/x/src/nextest.rs @@ -0,0 +1,142 @@ +// Copyright (c) The Diem Core Contributors +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + cargo::{ + build_args::{BuildArgs, Coloring}, + selected_package::SelectedPackageArgs, + CargoCommand, + }, + context::XContext, + Result, +}; +use anyhow::bail; +use cargo_metadata::Message; +use guppy::PackageId; +use std::ffi::OsString; +use structopt::StructOpt; +use testrunner::{ + reporter::{Color, ReporterOpts, TestReporter}, + runner::TestRunnerOpts, + test_filter::{RunIgnored, TestFilter}, + test_list::{TestBinary, TestList}, +}; + +#[derive(Debug, StructOpt)] +pub struct Args { + #[structopt(flatten)] + pub(crate) package_args: SelectedPackageArgs, + #[structopt(long, short)] + /// Skip running expensive diem testsuite integration tests + unit: bool, + #[structopt(flatten)] + pub(crate) build_args: BuildArgs, + #[structopt(long)] + /// Do not run tests, only compile the test executables + no_run: bool, + /// Run ignored tests + #[structopt(long, possible_values = &RunIgnored::variants(), default_value, case_insensitive = true)] + run_ignored: RunIgnored, + #[structopt(name = "FILTERS", last = true)] + filters: Vec, + #[structopt(flatten)] + reporter_opts: ReporterOpts, +} + +pub fn run(args: Args, xctx: XContext) -> Result<()> { + let config = xctx.config(); + + let mut packages = args.package_args.to_selected_packages(&xctx)?; + if args.unit { + packages.add_excludes(config.system_tests().iter().map(|(p, _)| p.as_str())); + } + + let mut direct_args = Vec::new(); + args.build_args.add_args(&mut direct_args); + + // Always pass in --no-run as the test runner is responsible for running these tests. + direct_args.push(OsString::from("--no-run")); + + // TODO: no-fail-fast (needs support in nextest) + + // Step 1: build all the test binaries with --no-run. + let cmd = CargoCommand::Test { + cargo_config: xctx.config().cargo_config(), + direct_args: direct_args.as_slice(), + // Don't pass in the args (test name) -- they're for use by the test runner. + args: &[], + env: &[], + }; + + let messages = cmd.run_capture_messages(&packages)?; + + if args.no_run { + // Don't proceed further. + return Ok(()); + } + + let package_graph = xctx.core().package_graph()?; + let workspace = package_graph.workspace(); + + let mut executables = vec![]; + for message in messages { + let message = message?; + match message { + Message::CompilerArtifact(artifact) if artifact.profile.test => { + if let Some(binary) = artifact.executable { + // Look up the executable by package ID. + let package_id = PackageId::new(artifact.package_id.repr); + + let package = package_graph.metadata(&package_id)?; + let cwd = Some( + workspace.root().join( + package + .source() + .workspace_path() + .expect("tests should never be built for non-workspace artifacts"), + ), + ); + + // Construct the friendly name from the package and build target. + let mut friendly_name = package.name().to_owned(); + if artifact.target.name != package.name() { + friendly_name.push_str("::"); + friendly_name.push_str(&artifact.target.name); + } + let friendly_name = Some(friendly_name); + + executables.push(TestBinary { + binary, + cwd, + friendly_name, + }); + } + } + _ => { + // Ignore all other messages. + } + } + } + + let test_filter = TestFilter::new(args.run_ignored, &args.filters); + let test_list = TestList::new(executables, &test_filter)?; + + let runner_opts = TestRunnerOpts { + jobs: args.build_args.jobs.map(|jobs| jobs as usize), + }; + let runner = runner_opts.build(&test_list); + + let color = match args.build_args.color { + Coloring::Auto => Color::Auto, + Coloring::Always => Color::Always, + Coloring::Never => Color::Never, + }; + let reporter = TestReporter::new(&test_list, color, args.reporter_opts); + + let run_stats = runner.try_execute(|event| reporter.report_event(event))?; + if !run_stats.is_success() { + bail!("test run failed"); + } + + Ok(()) +}