fix: honour a config rule's filterset instead of selecting every test

src/collect.rs

@@ -1038,7 +1038,8 @@ pub(crate) fn cargo_build_args(nextest_args: &[String]) -> Vec<String> {
 }
 
 /// Result of `cargo nextest list`: every testcase as a (binary_id, test_name)
-/// pair, the subset that is ignored, plus per-binary metadata.
+/// pair, the subset that is ignored, the subset the listing's own filterset
+/// rejected, plus per-binary metadata.
 pub(crate) struct Listing {
     /// Every testcase nextest enumerated, ignored or not. The complete set —
     /// `collect --diff` prunes DB rows against it, so a merely-ignored test
@@ -1049,6 +1050,28 @@ pub(crate) struct Listing {
     /// are skipped by `cargo nextest run`, so they never gain coverage;
     /// new-test detection must exclude them or they read as "new" forever.
     pub(crate) ignored: BTreeSet<TestId>,
+    /// Subset of `tests` that the `filter_expr` passed to this listing
+    /// rejected — nextest tags those `filter-match: { status: "mismatch",
+    /// reason: "expression" }`. Empty when no filterset was passed.
+    ///
+    /// `cargo nextest list -E <expr>` does *not* drop non-matching testcases
+    /// from `testcases`; it lists everything and tags each one. Reading
+    /// `tests` alone therefore treats "every test in the workspace" as the
+    /// filterset's result, which is why this subset has to be carried
+    /// separately rather than folded into `tests`.
+    ///
+    /// Keyed on `reason`, not on `status`: nextest reports an `#[ignore]`d
+    /// test as `mismatch`/`ignored` even with no `-E` at all, so keying on
+    /// `status` would silently pull ignored tests in here on every listing.
+    /// Other reasons (`string`, `default-filter`) are left out too — they are
+    /// not this filterset's verdict, and leaving them in `tests` keeps the
+    /// error on the over-selecting side.
+    ///
+    /// The flip side: `ignored` wins over `expression` when both apply, so an
+    /// ignored test the filterset *also* rejects is absent from here. A caller
+    /// reading this as "the filterset's tests are `tests` minus this set" has
+    /// to subtract [`Listing::ignored`] as well — see `resolve_config_hits`.
+    pub(crate) filterset_mismatched: BTreeSet<TestId>,
     pub(crate) binaries: Vec<BinaryEntry>,
 }
 
@@ -1077,9 +1100,11 @@ pub(crate) struct BinaryEntry {
 /// enumerates a different test set than the run builds and new-test
 /// detection ("listed minus DB") becomes unsound.
 ///
-/// `filter_expr`, when set, passes `-E <expr>` so the listing is restricted to
+/// `filter_expr`, when set, passes `-E <expr>` so the listing can name the
 /// tests matching a nextest filterset — used to resolve `[workspace.metadata.affected]`
-/// rules to concrete tests. Leave `None` for a full listing.
+/// rules to concrete tests. Leave `None` for a full listing. Note that `-E`
+/// does not *restrict* what nextest lists: rejected testcases still appear in
+/// `tests`, tagged, and surface as [`Listing::filterset_mismatched`].
 pub(crate) fn nextest_list(
     project_root: &Path,
     rustflags_override: Option<&str>,
@@ -1127,6 +1152,7 @@ pub(crate) fn nextest_list(
 
     let mut tests = BTreeSet::new();
     let mut ignored = BTreeSet::new();
+    let mut filterset_mismatched = BTreeSet::new();
     let mut binaries = Vec::new();
     if let Some(suites) = json.get("rust-suites").and_then(|v| v.as_object()) {
         for suite in suites.values() {
@@ -1155,17 +1181,36 @@ pub(crate) fn nextest_list(
                 if is_ignored {
                     ignored.insert(test_id.clone());
                 }
+                if filter_expression_rejected(case)? {
+                    filterset_mismatched.insert(test_id.clone());
+                }
                 tests.insert(test_id);
             }
         }
     }
     Ok(Listing {
         tests: tests.into_iter().collect(),
         ignored,
+        filterset_mismatched,
         binaries,
     })
 }
 
+/// Whether nextest rejected this testcase because of the `-E` filterset —
+/// `filter-match: { status: "mismatch", reason: "expression" }`.
+///
+/// A missing `filter-match` is an error rather than "not rejected": swallowing
+/// it would silently restore the whole-workspace over-selection this guards
+/// against. `require_nextest` gates on a version that always emits the field.
+fn filter_expression_rejected(case: &serde_json::Value) -> Result<bool> {
+    let fm = case
+        .get("filter-match")
+        .context("nextest list testcase missing `filter-match`")?;
+    let status = fm.get("status").and_then(|v| v.as_str());
+    let reason = fm.get("reason").and_then(|v| v.as_str());
+    Ok(status == Some("mismatch") && reason == Some("expression"))
+}
+
 /// Ensure `cargo nextest` is available and recent enough that it sets
 /// `NEXTEST_BINARY_ID` per test invocation (the runner shim relies on it
 /// to attribute coverage). Fails with an install hint otherwise.
@@ -1328,6 +1373,7 @@ mod tests {
         Listing {
             tests: tests.iter().map(|(b, t)| TestId::new(*b, *t)).collect(),
             ignored: BTreeSet::new(),
+            filterset_mismatched: BTreeSet::new(),
             binaries: binaries
                 .iter()
                 .map(|(id, path)| BinaryEntry {

src/config.rs

@@ -10,10 +10,11 @@
 //! with `cargo nextest list -E`, so it speaks the full nextest filter language.
 //!
 //! The rules ride in the manifest cargo-affected already loads via `cargo
-//! metadata`, so there's no extra file to read. The trade-off is that the
-//! manifest is fingerprinted: editing a rule changes the coverage-cache key, so
-//! the next run re-collects. No rules → the tool behaves exactly as before, with
-//! no extra `nextest list` invocation.
+//! metadata`, so there's no extra file to read. Manifests are fingerprinted,
+//! but [`crate::fingerprint`] strips `[*.metadata]` before hashing — cargo
+//! ignores it for builds — so editing a rule is cache-neutral and you can
+//! iterate on rules without re-collecting. No rules → the tool behaves exactly
+//! as before, with no extra `nextest list` invocation.
 
 use std::collections::{BTreeMap, BTreeSet};
 use std::path::Path;
@@ -159,6 +160,8 @@ pub(crate) fn config_rule_hits(
 /// For each rule with at least one matching changed path, `cargo nextest list
 /// -E <filterset>` resolves the filterset to concrete tests — using the same
 /// build flags as the run, so the listing matches what nextest will build.
+/// `-E` tags rather than filters (see [`crate::collect::Listing`]), so the
+/// rule's tests are the listing minus the testcases the filterset rejected.
 /// Keying on the changed path lets the JSON report attribute the selection to
 /// the file that triggered it. Rules with no matching path cost nothing (no
 /// nextest invocation), so a Rust-only diff is byte-for-byte the prior
@@ -190,7 +193,25 @@ pub(crate) fn resolve_config_hits(
                     rule.filterset
                 )
             })?;
-        let tests: BTreeSet<TestId> = listing.tests.into_iter().collect();
+        // `nextest list -E` enumerates everything and tags each testcase, so
+        // the filterset's actual result is the listing minus what it rejected.
+        // Reading `tests` alone would force-select every test in the workspace
+        // for any rule whose glob matched.
+        //
+        // Ignored tests come out too. nextest reports `ignored` as the mismatch
+        // *reason* in preference to `expression`, so an `#[ignore]`d test that
+        // the filterset also rejects is tagged `mismatch`/`ignored` and never
+        // lands in `filterset_mismatched` — without this, any single ignored
+        // test in the workspace would make every rule's resolved set non-empty
+        // and silence the typo'd-filterset warning below. They also can't be
+        // the answer: `nextest run` skips them, so force-selecting one selects
+        // nothing that runs.
+        let tests: BTreeSet<TestId> = listing
+            .tests
+            .iter()
+            .filter(|t| !listing.filterset_mismatched.contains(t) && !listing.ignored.contains(t))
+            .cloned()
+            .collect();
         if tests.is_empty() {
             eprintln!(
                 "warning: {TABLE} rule matched {} but its filterset ({:?}) \

tests/functional/config_rule.rs

@@ -157,6 +157,164 @@ fn config_rule_rescues_committed_added_input() {
     );
 }
 
+/// A bogus filterset in `[*.metadata.affected]` that nextest's parser rejects
+/// must error end-to-end, not get swallowed into a silent skip. The "fail
+/// loudly" principle (CLAUDE.md) is load-bearing here: a typo'd filterset that
+/// silently selected zero tests would re-open the very gap the rule exists to
+/// close.
+#[test]
+fn config_rule_bogus_filterset_fails_loudly() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    write_golden_project(dir);
+    // `&&` is a binary operator with no operands — nextest's filterset parser
+    // rejects it.
+    add_affected_rule(dir, "\"golden.txt\"", "&&");
+    init_git_with_initial_commit(dir);
+
+    // Seed coverage. `collect` doesn't resolve filtersets (no diff yet) so the
+    // bogus rule doesn't block the cache.
+    let collect = cargo_affected(dir, &["affected", "collect"]);
+    assert!(
+        collect.status.success(),
+        "collect failed: {}",
+        combined_output(&collect)
+    );
+
+    // Touch the rule's input. Resolution now happens — and must fail.
+    replace_in_file(&dir.join("golden.txt"), "hello", "hi");
+
+    let out = cargo_affected(dir, &["affected", "status", "-v"]);
+    let combined = combined_output(&out);
+    assert!(
+        !out.status.success(),
+        "expected failure for bogus filterset, got success: {combined}"
+    );
+    assert!(
+        combined.contains("filterset"),
+        "error must name the offending filterset: {combined}"
+    );
+}
+
+/// A rule's `filterset` must narrow the selection to the tests it names.
+///
+/// `cargo nextest list -E <expr>` lists *every* testcase and tags each with
+/// `filter-match`, rather than emitting only the matches. Reading the listing
+/// without consulting that tag made every rule force-select the whole
+/// workspace the moment one of its globs matched — safe, but it silently
+/// discards the entire speedup on any diff that touches a configured input,
+/// and it made the "filterset selected no tests" warning unreachable in any
+/// workspace with at least one test.
+#[test]
+fn config_rule_filterset_narrows_to_named_tests() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    write_golden_project(dir);
+    // A **second, unrelated** test. One test is not enough to see whether a
+    // rule's filterset is honoured: with a single-test crate, "the tests the
+    // filterset names" and "every test in the workspace" are the same set, so
+    // an implementation that ignored the filterset entirely would still look
+    // right. It reads nothing on disk, so nothing links it to `golden.txt`.
+    std::fs::write(
+        dir.join("tests").join("unrelated.rs"),
+        "#[test]\nfn unrelated_test() {\n    assert_eq!(config_rule_sample::GREETING, \"hello\");\n}\n",
+    )
+    .unwrap();
+    add_affected_rule(dir, "\"golden.txt\"", "test(=golden_matches)");
+    init_git_with_initial_commit(dir);
+
+    let collect = cargo_affected(dir, &["affected", "collect"]);
+    assert!(
+        collect.status.success(),
+        "collect failed: {}",
+        combined_output(&collect)
+    );
+
+    // Only the non-Rust input changes: coverage selects nothing, so every
+    // selected test comes from the rule.
+    replace_in_file(&dir.join("golden.txt"), "hello", "hi");
+
+    let out = combined_output(&cargo_affected(dir, &["affected", "status", "-v"]));
+    assert!(
+        out.contains("golden_matches (config)"),
+        "the filterset names golden_matches, so it must be config-selected: {out}"
+    );
+    assert!(
+        !out.contains("unrelated_test"),
+        "unrelated_test is outside the rule's filterset and must NOT be \
+         selected — the filterset is being ignored: {out}"
+    );
+    assert!(
+        out.contains("1 config"),
+        "exactly one test matches the filterset: {out}"
+    );
+    assert!(
+        out.contains("selection=1/2"),
+        "expected 1 of 2 tests selected: {out}"
+    );
+}
+
+/// A rule whose filterset is *valid* but resolves to zero tests must surface a
+/// warning. The fast path already returns `Ok(())` for an empty rule set; the
+/// risk is a typo'd test name (a valid filterset that simply matches nothing)
+/// silently selecting nothing for the changed input. The warning is the
+/// signal that the rule is no longer doing its job.
+///
+/// Unreachable until the filterset was actually honoured: `nextest list -E`
+/// enumerates every testcase, so the resolved set was never empty in a
+/// workspace with at least one test.
+///
+/// The crate carries an `#[ignore]`d test on purpose. nextest reports
+/// `ignored` as the mismatch *reason* in preference to `expression`, so an
+/// ignored test is never tagged as filterset-rejected — resolving the rule by
+/// "listing minus filterset-rejected" alone leaves it in, the set is non-empty,
+/// and this warning goes quiet again in any workspace with a single ignored
+/// test.
+#[test]
+fn config_rule_warns_when_filterset_matches_nothing() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    write_golden_project(dir);
+    std::fs::write(
+        dir.join("tests").join("ignored.rs"),
+        "#[test]\n#[ignore]\nfn ignored_test() {}\n",
+    )
+    .unwrap();
+    // Syntactically valid filterset; nextest accepts it and returns zero tests.
+    add_affected_rule(dir, "\"golden.txt\"", "test(=no_such_test_anywhere)");
+    init_git_with_initial_commit(dir);
+
+    let collect = cargo_affected(dir, &["affected", "collect"]);
+    assert!(
+        collect.status.success(),
+        "collect failed: {}",
+        combined_output(&collect)
+    );
+
+    replace_in_file(&dir.join("golden.txt"), "hello", "hi");
+
+    let out = cargo_affected(dir, &["affected", "status", "-v"]);
+    let combined = combined_output(&out);
+    // The command itself succeeds — an empty match isn't a hard error — but
+    // the warning must be visible so a typo can't silently reopen the gap.
+    assert!(
+        out.status.success(),
+        "an empty match is not a hard error: {combined}"
+    );
+    assert!(
+        combined.contains("selected no tests"),
+        "expected the no-tests warning to fire: {combined}"
+    );
+    assert!(
+        combined.contains("golden.txt"),
+        "warning should name the matched path: {combined}"
+    );
+    assert!(
+        combined.contains("selection=0/1"),
+        "an empty filterset must contribute nothing to the selection: {combined}"
+    );
+}
+
 /// A rule that matches no changed path must be inert: a Rust-only edit takes
 /// the exact pre-rule path, with no config category and no extra selection.
 #[test]