Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nextest-filtering] glob matcher and binary id predicates #1132

Merged
merged 4 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ 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
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/tests/integration/fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub static EXPECTED_LIST: Lazy<Vec<TestInfo>> = Lazy::new(|| {
],
),
TestInfo::new(
"nextest-derive::proc-macro/nextest-derive",
"nextest-derive",
BuildPlatform::Host,
vec![("it_works", false)],
),
Expand Down Expand Up @@ -419,7 +419,7 @@ pub fn check_run_output(stderr: &[u8], relocated: bool) {
(true, "nextest-tests::other other_test_success"),
(true, "nextest-tests::basic test_success"),
(false, "nextest-tests::segfault test_segfault"),
(true, "nextest-derive::proc-macro/nextest-derive it_works"),
(true, "nextest-derive it_works"),
(
true,
"nextest-tests::example/other tests::other_example_success",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ group: @global
tests::test_multiply_two_cdylib
cdylib-link:
test_multiply_two
nextest-derive::proc-macro/nextest-derive:
nextest-derive:
it_works
nextest-tests:
tests::call_dylib_add_two
Expand Down Expand Up @@ -46,4 +46,3 @@ group: @global
tests::example_success
nextest-tests::example/other:
tests::other_example_success

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ group: @global
tests::test_multiply_two_cdylib
cdylib-link:
test_multiply_two
nextest-derive::proc-macro/nextest-derive:
nextest-derive:
it_works
nextest-tests:
tests::call_dylib_add_two
Expand Down Expand Up @@ -47,4 +47,3 @@ group: @global
tests::example_success
nextest-tests::example/other:
tests::other_example_success

Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ group: @global
tests::test_multiply_two_cdylib
cdylib-link:
test_multiply_two
nextest-derive::proc-macro/nextest-derive:
nextest-derive:
it_works
nextest-tests:
tests::call_dylib_add_two
Expand Down Expand Up @@ -48,4 +48,3 @@ group: @global
tests::example_success
nextest-tests::example/other:
tests::other_example_success

2 changes: 2 additions & 0 deletions nextest-filtering/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal-testing = ["dep:proptest", "dep:test-strategy", "dep:twox-hash"]
# trace = ["nom-tracable/trace"]

[dependencies]
globset.workspace = true
guppy = "0.17.3"
miette = "5.10.0"
nom = "7.1.3"
Expand All @@ -35,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 }
Expand Down
1 change: 1 addition & 0 deletions nextest-filtering/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions nextest-filtering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ pub enum ParseSingleError {
message: String,
},

/// An invalid glob pattern was encountered.
#[error("invalid glob")]
InvalidGlob {
/// The part of the input that failed.
#[label("{}", error)]
span: SourceSpan,

/// The underlying error.
error: GlobConstructError,
},

/// An invalid regex was encountered but we couldn't determine a better error message.
#[error("invalid regex")]
InvalidRegexWithoutMessage(#[label("invalid regex")] SourceSpan),
Expand Down Expand Up @@ -123,6 +134,15 @@ impl ParseSingleError {
}
}

#[derive(Clone, Debug, Error, PartialEq, Eq)]
pub enum GlobConstructError {
#[error("{}", .0.kind())]
InvalidGlob(globset::Error),

#[error("{}", .0)]
RegexError(String),
}

#[derive(Debug, Clone)]
pub(crate) struct State<'a> {
// A `RefCell` is required here because the state must implement `Clone` to work with nom.
Expand Down
29 changes: 25 additions & 4 deletions nextest-filtering/src/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
use crate::{
errors::{FilterExpressionParseErrors, ParseSingleError, State},
parsing::{
parse, DisplayParsedRegex, DisplayParsedString, ExprResult, ParsedExpr, SetDef, Span,
parse, DisplayParsedRegex, DisplayParsedString, ExprResult, GenericGlob, ParsedExpr,
SetDef, Span,
},
};
use guppy::{
graph::{cargo::BuildPlatform, PackageGraph},
PackageId,
};
use miette::SourceSpan;
use nextest_metadata::{RustBinaryId, RustTestBinaryKind};
use recursion::{Collapsible, CollapsibleExt, MappableFrame, PartiallyApplied};
use std::{cell::RefCell, collections::HashSet, fmt};

Expand All @@ -24,6 +26,8 @@
Equal { value: String, implicit: bool },
/// Simple contains test
Contains { value: String, implicit: bool },
/// Test against a glob
Glob { glob: GenericGlob, implicit: bool },
/// Test against a regex
Regex(regex::Regex),
}
Expand Down Expand Up @@ -68,6 +72,9 @@
},
) => s1 == s2 && default1 == default2,
(Self::Regex(r1), Self::Regex(r2)) => r1.as_str() == r2.as_str(),
(Self::Glob { glob: g1, .. }, Self::Glob { glob: g2, .. }) => {
g1.regex().as_str() == g2.regex().as_str()
}
_ => false,
}
}
Expand All @@ -90,6 +97,12 @@
if *implicit { "" } else { "~" },
DisplayParsedString(value)
),
Self::Glob { glob, implicit } => write!(
f,
"{}{}",
if *implicit { "" } else { "#" },
DisplayParsedString(glob.as_str())
),
Self::Regex(r) => write!(f, "/{}/", DisplayParsedRegex(r)),
}
}
Expand All @@ -106,6 +119,8 @@
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
Expand All @@ -120,11 +135,14 @@
/// 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,
Expand Down Expand Up @@ -172,6 +190,7 @@
match self {
Self::Equal { value, .. } => value == input,
Self::Contains { value, .. } => input.contains(value),
Self::Glob { glob, .. } => glob.is_match(input),
Self::Regex(reg) => reg.is_match(input),
}
}
Expand All @@ -184,8 +203,9 @@
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),
}
}
Expand All @@ -196,8 +216,9 @@
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())),

Check warning on line 219 in nextest-filtering/src/expression.rs

View check run for this annotation

Codecov / codecov/patch

nextest-filtering/src/expression.rs#L219

Added line #L219 was not covered by tests
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)),
}
}
Expand Down
Loading