From c1cfe1879042dd1fe99fde502dfc2116838eee7e Mon Sep 17 00:00:00 2001 From: Rain Date: Sun, 26 Nov 2023 16:10:55 -0800 Subject: [PATCH] [nextest-filtering] add a "binary_id" predicate This predicate can be used to match against the binary ID, by copy-pasting it from the output. This can be done today with `package(foo) & binary(bar)` but is much easier this way. --- Cargo.lock | 2 +- Cargo.toml | 1 + nextest-filtering/Cargo.toml | 1 + nextest-filtering/src/compile.rs | 1 + nextest-filtering/src/expression.rs | 14 +- nextest-filtering/src/parsing.rs | 5 + nextest-filtering/tests/match.rs | 551 +++++++--------------- nextest-metadata/src/test_list.rs | 25 +- nextest-runner/src/config/overrides.rs | 45 +- nextest-runner/src/config/retry_policy.rs | 12 +- nextest-runner/src/config/scripts.rs | 32 +- nextest-runner/src/config/test_helpers.rs | 46 +- nextest-runner/src/config/tool_config.rs | 37 +- nextest-runner/src/list/binary_list.rs | 29 +- nextest-runner/src/list/test_list.rs | 14 +- nextest-runner/src/test_filter.rs | 21 +- workspace-hack/Cargo.toml | 1 - 17 files changed, 338 insertions(+), 499 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c27fa7d98a..413132a7104 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1657,6 +1657,7 @@ dependencies = [ "globset", "guppy", "miette", + "nextest-metadata", "nextest-workspace-hack", "nom", "nom-tracable", @@ -1799,7 +1800,6 @@ dependencies = [ "similar", "syn 1.0.109", "syn 2.0.39", - "target-spec", "tokio", "twox-hash", "uuid", diff --git a/Cargo.toml b/Cargo.toml index 977dedd5783..2e4eee05eed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ members = [ [workspace.dependencies] globset = "0.4.14" +nextest-metadata = { path = "nextest-metadata" } nextest-workspace-hack = "0.1.0" # make backtrace + color-eyre faster on debug builds diff --git a/nextest-filtering/Cargo.toml b/nextest-filtering/Cargo.toml index 3ddd0b19a10..1552c692895 100644 --- a/nextest-filtering/Cargo.toml +++ b/nextest-filtering/Cargo.toml @@ -36,6 +36,7 @@ recursion = "0.5.1" regex = "1.10.2" regex-syntax = "0.8.2" thiserror = "1.0.50" +nextest-metadata.workspace = true proptest = { version = "1.4.0", optional = true } test-strategy = { version = "0.3.1", optional = true } twox-hash = { version = "1.6.3", optional = true } diff --git a/nextest-filtering/src/compile.rs b/nextest-filtering/src/compile.rs index 53d95889c85..62a6b8555a7 100644 --- a/nextest-filtering/src/compile.rs +++ b/nextest-filtering/src/compile.rs @@ -109,6 +109,7 @@ fn compile_set_def( )), SetDef::Kind(matcher, span) => FilteringSet::Kind(matcher.clone(), *span), SetDef::Binary(matcher, span) => FilteringSet::Binary(matcher.clone(), *span), + SetDef::BinaryId(matcher, span) => FilteringSet::BinaryId(matcher.clone(), *span), SetDef::Platform(platform, span) => FilteringSet::Platform(*platform, *span), SetDef::Test(matcher, span) => FilteringSet::Test(matcher.clone(), *span), SetDef::All => FilteringSet::All, diff --git a/nextest-filtering/src/expression.rs b/nextest-filtering/src/expression.rs index 41d0efc8d31..b70959c29b7 100644 --- a/nextest-filtering/src/expression.rs +++ b/nextest-filtering/src/expression.rs @@ -13,6 +13,7 @@ use guppy::{ PackageId, }; use miette::SourceSpan; +use nextest_metadata::{RustBinaryId, RustTestBinaryKind}; use recursion::{Collapsible, CollapsibleExt, MappableFrame, PartiallyApplied}; use std::{cell::RefCell, collections::HashSet, fmt}; @@ -118,6 +119,8 @@ pub enum FilteringSet { Platform(BuildPlatform, SourceSpan), /// All binaries matching a name Binary(NameMatcher, SourceSpan), + /// All binary IDs matching a name + BinaryId(NameMatcher, SourceSpan), /// All tests matching a name Test(NameMatcher, SourceSpan), /// All tests @@ -132,11 +135,14 @@ pub struct BinaryQuery<'a> { /// The package ID. pub package_id: &'a PackageId, + /// The binary ID. + pub binary_id: &'a RustBinaryId, + /// The name of the binary. pub binary_name: &'a str, /// The kind of binary this test is (lib, test etc). - pub kind: &'a str, + pub kind: &'a RustTestBinaryKind, /// The platform this test is built for. pub platform: BuildPlatform, @@ -197,8 +203,9 @@ impl FilteringSet { Self::None => false, Self::Test(matcher, _) => matcher.is_match(query.test_name), Self::Binary(matcher, _) => matcher.is_match(query.binary_query.binary_name), + Self::BinaryId(matcher, _) => matcher.is_match(query.binary_query.binary_id.as_str()), Self::Platform(platform, _) => query.binary_query.platform == *platform, - Self::Kind(matcher, _) => matcher.is_match(query.binary_query.kind), + Self::Kind(matcher, _) => matcher.is_match(query.binary_query.kind.as_str()), Self::Packages(packages) => packages.contains(query.binary_query.package_id), } } @@ -209,8 +216,9 @@ impl FilteringSet { Self::None => Logic::bottom(), Self::Test(_, _) => None, Self::Binary(matcher, _) => Some(matcher.is_match(query.binary_name)), + Self::BinaryId(matcher, _) => Some(matcher.is_match(query.binary_id.as_str())), Self::Platform(platform, _) => Some(query.platform == *platform), - Self::Kind(matcher, _) => Some(matcher.is_match(query.kind)), + Self::Kind(matcher, _) => Some(matcher.is_match(query.kind.as_str())), Self::Packages(packages) => Some(packages.contains(query.package_id)), } } diff --git a/nextest-filtering/src/parsing.rs b/nextest-filtering/src/parsing.rs index 6e20cabda67..c8eb601d4e1 100644 --- a/nextest-filtering/src/parsing.rs +++ b/nextest-filtering/src/parsing.rs @@ -53,6 +53,7 @@ pub enum SetDef { Rdeps(NameMatcher, S), Kind(NameMatcher, S), Binary(NameMatcher, S), + BinaryId(NameMatcher, S), Platform(BuildPlatform, S), Test(NameMatcher, S), All, @@ -67,6 +68,7 @@ impl SetDef { Self::Rdeps(matcher, _) => SetDef::Rdeps(matcher, ()), Self::Kind(matcher, _) => SetDef::Kind(matcher, ()), Self::Binary(matcher, _) => SetDef::Binary(matcher, ()), + Self::BinaryId(matcher, _) => SetDef::BinaryId(matcher, ()), Self::Platform(platform, _) => SetDef::Platform(platform, ()), Self::Test(matcher, _) => SetDef::Test(matcher, ()), Self::All => SetDef::All, @@ -83,6 +85,7 @@ impl fmt::Display for SetDef { Self::Rdeps(matcher, _) => write!(f, "rdeps({matcher})"), Self::Kind(matcher, _) => write!(f, "kind({matcher})"), Self::Binary(matcher, _) => write!(f, "binary({matcher})"), + Self::BinaryId(matcher, _) => write!(f, "binary_id({matcher})"), Self::Platform(platform, _) => write!(f, "platform({platform})"), Self::Test(matcher, _) => write!(f, "test({matcher})"), Self::All => write!(f, "all()"), @@ -599,6 +602,8 @@ fn parse_set_def(input: Span) -> IResult> { unary_set_def("deps", DefaultMatcher::Glob, SetDef::Deps), unary_set_def("rdeps", DefaultMatcher::Glob, SetDef::Rdeps), unary_set_def("kind", DefaultMatcher::Equal, SetDef::Kind), + // binary_id must go above binary, otherwise we'll parse the opening predicate wrong. + unary_set_def("binary_id", DefaultMatcher::Glob, SetDef::BinaryId), unary_set_def("binary", DefaultMatcher::Glob, SetDef::Binary), unary_set_def("test", DefaultMatcher::Contains, SetDef::Test), platform_def, diff --git a/nextest-filtering/tests/match.rs b/nextest-filtering/tests/match.rs index 79b65a92133..7d8ae1578b1 100644 --- a/nextest-filtering/tests/match.rs +++ b/nextest-filtering/tests/match.rs @@ -12,6 +12,7 @@ use nextest_filtering::{ errors::{FilterExpressionParseErrors, ParseSingleError}, BinaryQuery, FilteringExpr, TestQuery, }; +use nextest_metadata::{RustBinaryId, RustTestBinaryKind}; use test_case::test_case; #[track_caller] @@ -35,6 +36,45 @@ fn parse(input: &str, graph: &PackageGraph) -> FilteringExpr { expr } +struct BinaryQueryCreator<'a> { + package_id: &'a PackageId, + binary_id: RustBinaryId, + kind: RustTestBinaryKind, + binary_name: &'a str, + platform: BuildPlatform, +} + +impl<'a> BinaryQueryCreator<'a> { + fn to_query(&self) -> BinaryQuery<'_> { + BinaryQuery { + package_id: self.package_id, + binary_id: &self.binary_id, + kind: &self.kind, + binary_name: self.binary_name, + platform: self.platform, + } + } +} + +fn binary_query<'a>( + graph: &'a PackageGraph, + package_id: &'a PackageId, + kind: &str, + binary_name: &'a str, + platform: BuildPlatform, +) -> BinaryQueryCreator<'a> { + let package_name = graph.metadata(package_id).unwrap().name(); + let kind = RustTestBinaryKind::new(kind.to_owned()); + let binary_id = RustBinaryId::from_parts(package_name, &kind, binary_name); + BinaryQueryCreator { + package_id, + binary_id, + kind, + binary_name, + platform, + } +} + #[test] fn test_expr_package_contains() { let graph = load_graph(); @@ -44,30 +84,18 @@ fn test_expr_package_contains() { let pid_b = mk_pid('b'); let pid_c = mk_pid('c'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_c, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -81,30 +109,18 @@ fn test_expr_package_equal() { let pid_b = mk_pid('b'); let pid_c = mk_pid('c'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_c, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -118,30 +134,43 @@ fn test_expr_package_regex() { let pid_b = mk_pid('b'); let pid_c = mk_pid('c'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_c, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), + test_name: "test_something" + })); +} + +#[test] +fn test_expr_binary_id_glob() { + let graph = load_graph(); + let expr = parse("binary_id(crate-[ab])", &graph); + + let pid_a = mk_pid('a'); + let pid_b = mk_pid('b'); + let pid_c = mk_pid('c'); + assert!(expr.matches_test(&TestQuery { + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), + test_name: "test_something" + })); + assert!(expr.matches_test(&TestQuery { + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), + test_name: "test_something" + })); + assert!(!expr.matches_test(&TestQuery { + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -160,75 +189,40 @@ fn test_expr_deps() { let pid_g = mk_pid('g'); // a-d are deps of d assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_c, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_d, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_d, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); // e-g are not deps of d assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_e, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_e, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_f, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_f, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_g, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_g, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -247,75 +241,40 @@ fn test_expr_rdeps() { let pid_g = mk_pid('g'); // a-c are not rdeps of d assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_c, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_c, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); // d-g are rdeps of d assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_d, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_d, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_e, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_e, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_f, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_f, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_g, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_g, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -352,33 +311,18 @@ fn test_expr_kind() { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "test", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "test", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib2", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib2", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -390,33 +334,18 @@ fn test_expr_binary() { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "test", - binary_name: "my-binary2", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "test", "my-binary2", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib2", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib2", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); } @@ -428,23 +357,13 @@ fn test_expr_platform() { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Host, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Host) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); @@ -452,23 +371,13 @@ fn test_expr_platform() { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Host, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Host) + .to_query(), test_name: "test_something" })); } @@ -480,23 +389,13 @@ fn test_expr_kind_partial() { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "test", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "test", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_something" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); } @@ -510,33 +409,18 @@ fn test_expr_test() { let pid_b = mk_pid('b'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_b, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_run" })); } @@ -548,23 +432,13 @@ fn test_expr_test_not() { let pid_a = mk_pid('a'); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_run" })); } @@ -578,33 +452,18 @@ fn test_expr_test_union(input: &str) { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_run" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_build" })); } @@ -617,33 +476,18 @@ fn test_expr_test_difference(input: &str) { let pid_a = mk_pid('a'); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse_set" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse_expr" })); } @@ -656,33 +500,18 @@ fn test_expr_test_intersect(input: &str) { let pid_a = mk_pid('a'); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse" })); assert!(!expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_expr" })); assert!(expr.matches_test(&TestQuery { - binary_query: BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, - + binary_query: binary_query(&graph, &pid_a, "lib", "my-binary", BuildPlatform::Target) + .to_query(), test_name: "test_parse_expr" })); } @@ -699,63 +528,45 @@ fn test_binary_query() { let pid_b = mk_pid('b'); // binary = foo should match the first predicate (pid_a should not be relevant). assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "foo", - platform: BuildPlatform::Target, - }), + expr.matches_binary( + &binary_query(&graph, &pid_a, "lib", "foo", BuildPlatform::Target).to_query() + ), Some(true) ); // platform = host should match the second predicate. assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "bar", - platform: BuildPlatform::Host, - }), + expr.matches_binary( + &binary_query(&graph, &pid_b, "lib", "bar", BuildPlatform::Host).to_query() + ), Some(true) ); // kind = bench should match the third predicate. assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_a, - kind: "bench", - binary_name: "baz", - platform: BuildPlatform::Target, - }), + expr.matches_binary( + &binary_query(&graph, &pid_b, "bench", "baz", BuildPlatform::Target).to_query() + ), Some(true) ); // This should result in an unknown result since it involves a test predicate. assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_a, - kind: "lib", - binary_name: "baz", - platform: BuildPlatform::Target, - }), + expr.matches_binary( + &binary_query(&graph, &pid_a, "lib", "baz", BuildPlatform::Target,).to_query() + ), None ); // This should not result in an unknown result because no matter what the test predicate is, // kind(bin) resolves to true. assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_a, - kind: "bin", - binary_name: "baz", - platform: BuildPlatform::Target, - }), + expr.matches_binary( + &binary_query(&graph, &pid_a, "bin", "baz", BuildPlatform::Target,).to_query() + ), Some(true) ); // This should result in Some(false) since it doesn't match anything. assert_eq!( - expr.matches_binary(&BinaryQuery { - package_id: &pid_b, - kind: "lib", - binary_name: "baz", - platform: BuildPlatform::Target, - }), + expr.matches_binary( + &binary_query(&graph, &pid_b, "lib", "baz", BuildPlatform::Target,).to_query() + ), Some(false) ); } diff --git a/nextest-metadata/src/test_list.rs b/nextest-metadata/src/test_list.rs index d379b446a61..103658e3c36 100644 --- a/nextest-metadata/src/test_list.rs +++ b/nextest-metadata/src/test_list.rs @@ -9,7 +9,7 @@ use std::{ borrow::Cow, cmp::Ordering, collections::{BTreeMap, BTreeSet}, - fmt, + fmt::{self, Write as _}, path::PathBuf, process::Command, }; @@ -279,6 +279,29 @@ impl RustBinaryId { Self(id.into()) } + /// Creates a new `RustBinaryId` from its constituent parts. + pub fn from_parts(package_name: &str, kind: &RustTestBinaryKind, name: &str) -> Self { + let mut id = package_name.to_owned(); + // To ensure unique binary IDs, we use the following scheme: + if kind == &RustTestBinaryKind::LIB { + // 1. If the target is a lib, use the package name. There can only be one + // lib per package, so this will always be unique. + } else if kind == &RustTestBinaryKind::TEST { + // 2. For integration tests, use the target name. Cargo enforces unique + // names for the same kind of targets in a package, so these will always + // be unique. + id.push_str("::"); + id.push_str(&name); + } else { + // 3. For all other target kinds, use a combination of the target kind and + // the target name. For the same reason as above, these will always be + // unique. + write!(id, "::{kind}/{name}").unwrap(); + } + + Self(id.into()) + } + /// Returns the identifier as a string. #[inline] pub fn as_str(&self) -> &str { diff --git a/nextest-runner/src/config/overrides.rs b/nextest-runner/src/config/overrides.rs index cd1d4a32137..bb24a6680c9 100644 --- a/nextest-runner/src/config/overrides.rs +++ b/nextest-runner/src/config/overrides.rs @@ -639,7 +639,6 @@ mod tests { use camino::Utf8Path; use camino_tempfile::tempdir; use indoc::indoc; - use nextest_filtering::BinaryQuery; use std::num::NonZeroUsize; use test_case::test_case; @@ -710,13 +709,10 @@ mod tests { .apply_build_platforms(&build_platforms()); // This query matches override 2. + let host_binary_query = + binary_query(&graph, package_id, "lib", "my-binary", BuildPlatform::Host); let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Host, - }, + binary_query: host_binary_query.to_query(), test_name: "test", }; let overrides = profile.settings_for(&query); @@ -743,13 +739,15 @@ mod tests { } // This query matches override 1 and 2. + let target_binary_query = binary_query( + &graph, + package_id, + "lib", + "my-binary", + BuildPlatform::Target, + ); let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "test", }; let overrides = profile.settings_for(&query); @@ -788,12 +786,7 @@ mod tests { // This query matches override 3. let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "override3", }; let overrides = profile.settings_for(&query); @@ -801,12 +794,7 @@ mod tests { // This query matches override 5. let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "override5", }; let overrides = profile.settings_for(&query); @@ -814,12 +802,7 @@ mod tests { // This query does not match any overrides. let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "no_match", }; let overrides = profile.settings_for(&query); diff --git a/nextest-runner/src/config/retry_policy.rs b/nextest-runner/src/config/retry_policy.rs index de1d63a848c..37e324e0acd 100644 --- a/nextest-runner/src/config/retry_policy.rs +++ b/nextest-runner/src/config/retry_policy.rs @@ -167,7 +167,7 @@ mod tests { use super::*; use crate::{ config::{ - test_helpers::{build_platforms, temp_workspace}, + test_helpers::{binary_query, build_platforms, temp_workspace}, NextestConfig, }, errors::ConfigParseErrorKind, @@ -177,7 +177,7 @@ mod tests { use config::ConfigError; use guppy::graph::cargo::BuildPlatform; use indoc::indoc; - use nextest_filtering::{BinaryQuery, TestQuery}; + use nextest_filtering::TestQuery; use test_case::test_case; #[test] @@ -626,13 +626,9 @@ mod tests { &Default::default(), ) .unwrap(); + let binary_query = binary_query(&graph, package_id, "lib", "my-binary", build_platform); let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: build_platform, - }, + binary_query: binary_query.to_query(), test_name: "my_test", }; let settings_for = config diff --git a/nextest-runner/src/config/scripts.rs b/nextest-runner/src/config/scripts.rs index 7831722f9ae..9ac41e19a9f 100644 --- a/nextest-runner/src/config/scripts.rs +++ b/nextest-runner/src/config/scripts.rs @@ -623,7 +623,6 @@ mod tests { use display_error_chain::DisplayErrorChain; use indoc::indoc; use maplit::btreeset; - use nextest_filtering::BinaryQuery; use test_case::test_case; #[test] @@ -710,13 +709,10 @@ mod tests { .apply_build_platforms(&build_platforms()); // This query matches the foo and bar scripts. + let host_binary_query = + binary_query(&graph, package_id, "lib", "my-binary", BuildPlatform::Host); let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Host, - }, + binary_query: host_binary_query.to_query(), test_name: "script1", }; let scripts = SetupScripts::new_with_queries(&profile, std::iter::once(query)); @@ -732,14 +728,17 @@ mod tests { "second script should be bar" ); + let target_binary_query = binary_query( + &graph, + package_id, + "lib", + "my-binary", + BuildPlatform::Target, + ); + // This query matches the baz script. let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "script2", }; let scripts = SetupScripts::new_with_queries(&profile, std::iter::once(query)); @@ -752,12 +751,7 @@ mod tests { // This query matches the baz, foo and tool scripts (but note the order). let query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: target_binary_query.to_query(), test_name: "script3", }; let scripts = SetupScripts::new_with_queries(&profile, std::iter::once(query)); diff --git a/nextest-runner/src/config/test_helpers.rs b/nextest-runner/src/config/test_helpers.rs index 8b8dcdcd1ba..6c86ccd6e6f 100644 --- a/nextest-runner/src/config/test_helpers.rs +++ b/nextest-runner/src/config/test_helpers.rs @@ -7,7 +7,12 @@ use crate::{ platform::BuildPlatforms, }; use camino::{Utf8Path, Utf8PathBuf}; -use guppy::{graph::PackageGraph, MetadataCommand}; +use guppy::{ + graph::{cargo::BuildPlatform, PackageGraph}, + MetadataCommand, PackageId, +}; +use nextest_filtering::BinaryQuery; +use nextest_metadata::{RustBinaryId, RustTestBinaryKind}; use serde::Deserialize; use std::{io::Write, path::PathBuf, process::Command}; use target_spec::{Platform, TargetFeatures}; @@ -39,6 +44,45 @@ pub(super) fn cargo_path() -> Utf8PathBuf { } } +pub(super) struct BinaryQueryCreator<'a> { + package_id: &'a PackageId, + binary_id: RustBinaryId, + kind: RustTestBinaryKind, + binary_name: &'a str, + platform: BuildPlatform, +} + +impl<'a> BinaryQueryCreator<'a> { + pub(super) fn to_query(&self) -> BinaryQuery<'_> { + BinaryQuery { + package_id: self.package_id, + binary_id: &self.binary_id, + kind: &self.kind, + binary_name: self.binary_name, + platform: self.platform, + } + } +} + +pub(super) fn binary_query<'a>( + graph: &'a PackageGraph, + package_id: &'a PackageId, + kind: &str, + binary_name: &'a str, + platform: BuildPlatform, +) -> BinaryQueryCreator<'a> { + let package_name = graph.metadata(package_id).unwrap().name(); + let kind = RustTestBinaryKind::new(kind.to_owned()); + let binary_id = RustBinaryId::from_parts(package_name, &kind, binary_name); + BinaryQueryCreator { + package_id, + binary_id, + kind, + binary_name, + platform, + } +} + pub(super) fn build_platforms() -> BuildPlatforms { BuildPlatforms::new_with_host( Platform::new("x86_64-unknown-linux-gnu", TargetFeatures::Unknown).unwrap(), diff --git a/nextest-runner/src/config/tool_config.rs b/nextest-runner/src/config/tool_config.rs index 64c5903a9b4..afbac3c3216 100644 --- a/nextest-runner/src/config/tool_config.rs +++ b/nextest-runner/src/config/tool_config.rs @@ -62,7 +62,7 @@ mod tests { }; use camino_tempfile::tempdir; use guppy::graph::cargo::BuildPlatform; - use nextest_filtering::{BinaryQuery, TestQuery}; + use nextest_filtering::TestQuery; #[test] fn parse_tool_config_file() { @@ -226,40 +226,27 @@ mod tests { let package_id = graph.workspace().iter().next().unwrap().id(); + let binary_query = binary_query( + &graph, + package_id, + "lib", + "my-binary", + BuildPlatform::Target, + ); let test_foo_query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query.to_query(), test_name: "test_foo", }; let test_bar_query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query.to_query(), test_name: "test_bar", }; let test_baz_query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query.to_query(), test_name: "test_baz", }; let test_quux_query = TestQuery { - binary_query: BinaryQuery { - package_id, - kind: "lib", - binary_name: "my-binary", - platform: BuildPlatform::Target, - }, + binary_query: binary_query.to_query(), test_name: "test_quux", }; diff --git a/nextest-runner/src/list/binary_list.rs b/nextest-runner/src/list/binary_list.rs index c9b2f161ba1..607b5e34cda 100644 --- a/nextest-runner/src/list/binary_list.rs +++ b/nextest-runner/src/list/binary_list.rs @@ -15,7 +15,7 @@ use nextest_metadata::{ RustNonTestBinarySummary, RustTestBinaryKind, RustTestBinarySummary, }; use owo_colors::OwoColorize; -use std::{fmt::Write as _, io, io::Write}; +use std::{io, io::Write}; /// A Rust test binary built by Cargo. #[derive(Clone, Debug)] @@ -195,15 +195,14 @@ impl<'g> BinaryListBuildState<'g> { let package_id = artifact.package_id.repr; // Look up the executable by package ID. + + let name = artifact.target.name; + let package = self .graph .metadata(&guppy::PackageId::new(package_id.clone())) .map_err(FromMessagesError::PackageGraph)?; - // Construct the binary ID from the package and build target. - let mut id = package.name().to_owned(); - let name = artifact.target.name; - let kind = artifact.target.kind; if kind.is_empty() { return Err(FromMessagesError::MissingTargetKind { @@ -231,24 +230,8 @@ impl<'g> BinaryListBuildState<'g> { ) }; - // To ensure unique binary IDs, we use the following scheme: - if computed_kind == RustTestBinaryKind::LIB { - // 1. If the target is a lib, use the package name. There can only be one - // lib per package, so this will always be unique. - } else if computed_kind == RustTestBinaryKind::TEST { - // 2. For integration tests, use the target name. Cargo enforces unique - // names for the same kind of targets in a package, so these will always - // be unique. - id.push_str("::"); - id.push_str(&name); - } else { - // 3. For all other target kinds, use a combination of the target kind and - // the target name. For the same reason as above, these will always be - // unique. - write!(id, "::{computed_kind}/{name}").unwrap(); - } - - let id = RustBinaryId::new(&id); + // Construct the binary ID from the package and build target. + let id = RustBinaryId::from_parts(package.name(), &computed_kind, &name); self.rust_binaries.push(RustTestBinary { path, diff --git a/nextest-runner/src/list/test_list.rs b/nextest-runner/src/list/test_list.rs index 6696d31dc4a..66af271a5b8 100644 --- a/nextest-runner/src/list/test_list.rs +++ b/nextest-runner/src/list/test_list.rs @@ -145,6 +145,17 @@ impl<'g> RustTestArtifact<'g> { Ok(binaries) } + /// Returns a [`BinaryQuery`] corresponding to this test artifact. + pub fn to_binary_query(&self) -> BinaryQuery<'_> { + BinaryQuery { + package_id: self.package.id(), + binary_id: &self.binary_id, + kind: &self.kind, + binary_name: &self.binary_name, + platform: convert_build_platform(self.build_platform), + } + } + // --- // Helper methods // --- @@ -886,7 +897,8 @@ impl<'a> TestInstance<'a> { TestQuery { binary_query: BinaryQuery { package_id: self.suite_info.package.id(), - kind: self.suite_info.kind.as_str(), + binary_id: &self.suite_info.binary_id, + kind: &self.suite_info.kind, binary_name: &self.suite_info.binary_name, platform: convert_build_platform(self.suite_info.build_platform), }, diff --git a/nextest-runner/src/test_filter.rs b/nextest-runner/src/test_filter.rs index 0ecc72ce860..5fcf623cef5 100644 --- a/nextest-runner/src/test_filter.rs +++ b/nextest-runner/src/test_filter.rs @@ -11,12 +11,11 @@ use crate::{ errors::TestFilterBuilderError, - helpers::convert_build_platform, list::RustTestArtifact, partition::{Partitioner, PartitionerBuilder}, }; use aho_corasick::AhoCorasick; -use nextest_filtering::{BinaryQuery, FilteringExpr, TestQuery}; +use nextest_filtering::{FilteringExpr, TestQuery}; use nextest_metadata::{FilterMatch, MismatchReason}; /// Whether to run ignored tests. @@ -114,19 +113,16 @@ impl TestFilterBuilder { /// This method is implemented directly on `TestFilterBuilder`. The statefulness of `TestFilter` /// is only used for counted test partitioning, and is not currently relevant for binaries. pub fn should_obtain_test_list_from_binary(&self, test_binary: &RustTestArtifact<'_>) -> bool { - let query = BinaryQuery { - package_id: test_binary.package.id(), - kind: test_binary.kind.as_str(), - binary_name: &test_binary.binary_name, - platform: convert_build_platform(test_binary.build_platform), - }; if self.exprs.is_empty() { // No expressions means match all tests. return true; } for expr in &self.exprs { // If this is a definite or probable match, then we should run this binary - if expr.matches_binary(&query).unwrap_or(true) { + if expr + .matches_binary(&test_binary.to_binary_query()) + .unwrap_or(true) + { return true; } } @@ -248,12 +244,7 @@ impl<'filter> TestFilter<'filter> { test_name: &str, ) -> FilterNameMatch { let query = TestQuery { - binary_query: BinaryQuery { - package_id: test_binary.package.id(), - kind: test_binary.kind.as_str(), - binary_name: &test_binary.binary_name, - platform: convert_build_platform(test_binary.build_platform), - }, + binary_query: test_binary.to_binary_query(), test_name, }; if self.builder.exprs.is_empty() { diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 6276a2c9689..3aacb0cdb4a 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -33,7 +33,6 @@ rand = { version = "0.8.5" } serde = { version = "1.0.193", features = ["alloc", "derive"] } serde_json = { version = "1.0.108", features = ["preserve_order", "unbounded_depth"] } similar = { version = "2.3.0", features = ["inline", "unicode"] } -target-spec = { version = "3.0.1", default-features = false, features = ["custom", "summaries"] } tokio = { version = "1.34.0", features = ["fs", "io-util", "macros", "process", "rt-multi-thread", "signal", "sync", "time", "tracing"] } twox-hash = { version = "1.6.3" } uuid = { version = "1.6.1", features = ["v4"] }