diff --git a/CLAUDE.md b/CLAUDE.md index 60750d8..1907236 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,9 +19,10 @@ the functional test suite. - `collect.rs` — Coverage pipeline: `cargo nextest list` → instrumented `nextest run` via the runner shim → SQLite write. Extraction is **inlined into the shim** (see `shim.rs`): each test's shim merges/exports/parses its own profraw and deletes the bundle before exiting, so peak disk usage is bounded by nextest's own concurrency (O(test-threads × per-test size) instead of O(whole-suite size)) with no external watcher or completion heuristic. After `nextest run` exits, `collect` reads the per-test `TestResult` files the shims wrote under `CARGO_AFFECTED_RESULTS_DIR`, folds in each test's crate-root sentinels, and writes the rows. `--diff` reuses the same pipeline, restricting the run to the affected + new test set. Also owns the test-selection plumbing shared with `run.rs`: `nextest_filter_expr` builds the filterset, `write_nextest_config` hands it to nextest as a `default-filter` in a generated config file (passed via `--config-file`) so an arbitrarily large selection never overflows the OS command-line limit. - `shim.rs` — Hidden `runner-shim` invoked per test by nextest. Sets a per-test `LLVM_PROFILE_FILE`, spawns and waits for the real binary, then extracts its coverage (`llvm-profdata`/`llvm-cov`), writes a `TestResult` JSON file, and deletes the per-test profraw dir. Waiting (rather than `execvp`) is what lets extraction run in-process; nextest signals each test's process group on cancellation, so the spawned child is reaped without the shim forwarding signals. - `coverage.rs` — Parses `llvm-cov export` JSON into `(file, line_start, line_end)` ranges per hit function. -- `fingerprint.rs` — SHA-256 of `Cargo.lock`, workspace `Cargo.toml`s, `rustc -vV`, `RUSTFLAGS`, `CARGO_BUILD_TARGET`. Queries scoped to the current fingerprint naturally miss when any tracked input changes — no explicit invalidation path. +- `fingerprint.rs` — SHA-256 of `Cargo.lock`, workspace `Cargo.toml`s, `rustc -vV`, `RUSTFLAGS`, `CARGO_BUILD_TARGET`. Queries scoped to the current fingerprint naturally miss when any tracked input changes — no explicit invalidation path. `[*.metadata]` tables are stripped from each manifest before hashing (cargo ignores them for builds), so editing `[workspace.metadata.affected]` rules is cache-neutral; metadata-free manifests hash by raw bytes so there's no churn on upgrade. - `db.rs` — SQLite at `target/affected/coverage.db`. `test_regions` rows carry a per-row `collect_sha` so `--diff` can leave unaffected tests anchored at their original sha while re-anchoring rerun tests at the new HEAD; multiple shas can coexist for one fingerprint. Diverged-sha rows linger until `cargo affected clean`. Crate roots ride the same table with sentinel `(1, i64::MAX)` ranges (the structural-edit backstop). Legacy schemas drop on open. -- `selection.rs` — Shared between `run`, `status`, and `collect --diff`. Owns reachability classification (`check_shas_reachable`), per-sha diff collection (`changed_ranges_per_sha`), the divergence notice, and the affected + new + listed selection itself. +- `selection.rs` — Shared between `run`, `status`, and `collect --diff`. Owns reachability classification (`check_shas_reachable`), per-sha diff collection (`changed_ranges_per_sha`), the divergence notice, and the selection itself: affected + config + new + stranded, where config hits (from `config.rs`) are a disjoint category that never inflates the coverage-overlap counts. +- `config.rs` — Declarative input→test rules from `[workspace.metadata.affected]` (or `[package.metadata.affected]` for single-crate projects). Each `[[rule]]` pairs input globs with a nextest filterset; when a changed path matches, `cargo nextest list -E` resolves the filterset and those tests are force-selected. Closes the blind spot where a test reads a non-Rust file at runtime (an insta `.snap`, a doc `.md`) that has no coverage row, so a change to it would otherwise select no test. No rules means no extra `nextest list` call. - `run.rs` — `collect_shas` → reachability → per-sha `git diff -U0` → selection → `nextest run` against the generated filter config. Widens to all tests only when every sha is diverged. - `status.rs` — Dry-run variant of `run`. diff --git a/Cargo.lock b/Cargo.lock index 94d8f31..8035d4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14,6 +14,15 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "1.0.0" @@ -85,6 +94,16 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bstr" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" +dependencies = [ + "memchr", + "serde", +] + [[package]] name = "camino" version = "1.2.2" @@ -101,6 +120,7 @@ dependencies = [ "anyhow", "camino", "clap", + "globset", "rusqlite", "serde", "serde_json", @@ -269,6 +289,19 @@ dependencies = [ "wasip3", ] +[[package]] +name = "globset" +version = "0.4.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52dfc19153a48bde0cbd630453615c8151bce3a5adfac7a0aebfbf0a1e1f57e3" +dependencies = [ + "aho-corasick", + "bstr", + "log", + "regex-automata", + "regex-syntax", +] + [[package]] name = "hashbrown" version = "0.14.5" @@ -431,6 +464,23 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "regex-automata" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" + [[package]] name = "rusqlite" version = "0.31.0" diff --git a/Cargo.toml b/Cargo.toml index db0139e..752a65c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ path = "src/main.rs" anyhow = "1" camino = { version = "1", features = ["serde1"] } clap = { version = "4", features = ["derive"] } +globset = "0.4" rusqlite = { version = "0.31", features = ["bundled"] } serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/README.md b/README.md index ea069cb..9e1e84b 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,9 @@ CI should still run the full suite. ### False negatives (tests skipped that should have run) - **Non-Rust sources.** `include_str!` / `include_bytes!` targets, SQL - files, migrations, assets, and templates aren't seen by llvm-cov. + files, migrations, assets, snapshots, and templates aren't seen by + llvm-cov — a change confined to one selects no test. [Input + rules](#input-rules) close this for inputs you can name. - **Build-time inputs not in the fingerprint.** The fingerprint covers `Cargo.lock`, workspace `Cargo.toml`s, `rustc -vV`, `RUSTFLAGS`, and `CARGO_BUILD_TARGET`. Changes to `build.rs`, `rust-toolchain.toml`, or @@ -128,6 +130,46 @@ CI should still run the full suite. When in doubt, `cargo affected collect` to refresh coverage, or skip cargo-affected and run the full suite. +## Input rules + +Coverage can't link a test to a non-Rust input it reads at runtime — an insta +`.snap`, a doc a sync-test compares against, an `include_str!` target — so a +change confined to that input selects no test (see [false +negatives](#false-negatives-tests-skipped-that-should-have-run)). Optional +`[[workspace.metadata.affected.rule]]` tables in `Cargo.toml` close the gap by +mapping input globs to the tests that depend on them (use +`[[package.metadata.affected.rule]]` in a single-crate project): + +```toml +# Any `.snap` edit re-runs the integration suite that owns the snapshots. +[[workspace.metadata.affected.rule]] +globs = ["**/*.snap"] +filterset = "binary_id(=mycrate::integration)" + +# Doc-sync tests read these inputs at runtime; run that module when any change. +[[workspace.metadata.affected.rule]] +globs = ["README.md", "docs/**/*.md"] +filterset = "test(/readme_sync/)" +``` + +Each rule pairs `globs` (matched against changed paths) with a nextest +`filterset` (the full [filter-expression +language](https://nexte.st/docs/filtersets/)). When a changed path matches, the +filterset is resolved with `cargo nextest list -E` and its tests are +force-selected — reported under a `config` category distinct from coverage-driven +selection. A Rust-only diff matches no globs and takes the exact prior path, so +the speedup is preserved; the extra `nextest list` runs only on diffs that touch +a configured input. A rule that matches a path but resolves to no tests warns +rather than failing silently. No rules → no change in behavior. + +The rules live in `[*.metadata]`, which cargo ignores for the build — so +cargo-affected excludes it from the coverage fingerprint. Editing a rule is +cache-neutral: it doesn't force a re-collect, so you can iterate on rules freely. + +Rules are a remedy of last resort, not a substitute for coverage: prefer letting +`collect` map Rust changes. Reach for a rule only for inputs llvm-cov +structurally cannot see, and keep periodic full runs for everything else. + ## Comparison with similar tools The biggest design choice is *how* a tool decides what changed. The headline diff --git a/docs/report-json.md b/docs/report-json.md index 69740e9..de6e058 100644 --- a/docs/report-json.md +++ b/docs/report-json.md @@ -125,6 +125,7 @@ and its `differing_labels` answers "which input changed?". { "selected": 2577, "affected": 2569, + "config": 0, "new": 6, "stranded": 2, "skipped": 924, @@ -136,11 +137,12 @@ and its `differing_labels` answers "which input changed?". | Field | Notes | |---|---| | `mode` | `"selection"` or `"full-suite-no-listing"`. | -| `selected` | Union of affected + new + stranded. `null` on full-suite paths. | +| `selected` | Union of affected + config + new + stranded. `null` on full-suite paths. | | `affected` | In DB at a reachable sha AND a hunk overlapped a stored row. | +| `config` | Reachable-known and force-selected by a `[workspace.metadata.affected]` input rule (would otherwise have been skipped). Disjoint from `affected`. | | `new` | Listed in nextest but not in the DB at all under this fingerprint. | | `stranded` | In DB but only at currently-missing collect_shas. | -| `skipped` | `total_reachable_known - affected` (saturating). | +| `skipped` | `total_reachable_known - affected - config` (saturating). | | `total_reachable_known` | Distinct test count under the fingerprint at reachable shas. | ### `selection.changed_files[]` @@ -156,20 +158,24 @@ Sorted `(tests_pulled_total desc, path asc)`. "tests_pulled_by_reason": { "line_overlap": 180, "structural_backstop": 0, - "crate_root_sentinel": 1060 + "crate_root_sentinel": 1060, + "config_rule": 0 } } ``` `tracked_by_coverage` is `true` iff the file has at least one stored `test_regions` row at a reachable sha. Non-Rust files (`.snap`, configs) -read `false`. +read `false` — and are exactly where `config_rule` selections show up. `tests_pulled_by_reason` is deduplicated by **strongest reason** per test per file: a test pulled in by both line overlap and a sentinel counts once, classified by the strongest reason -(`line_overlap` > `structural_backstop` > `crate_root_sentinel`). The -three counters sum to `tests_pulled_total`. +(`line_overlap` > `structural_backstop` > `config_rule` > +`crate_root_sentinel`). `config_rule` counts tests force-selected by a +`[workspace.metadata.affected]` rule matching this path (non-zero only for the +non-Rust inputs such rules target). The four counters sum to +`tests_pulled_total`. ### `selection.selected_tests[]` @@ -195,10 +201,11 @@ Sorted `(binary_id, test_name)`. | Field | Notes | |---|---| -| `kind` | `"affected"`, `"new"`, or `"stranded"`. | -| `reasons` | Empty for `"new"` and `"stranded"`. Sorted `(file, kind, collect_sha)`. | -| `reasons[].kind` | `"line_overlap"`, `"structural_backstop"`, or `"crate_root_sentinel"`. | -| `reasons[].stored_range` | `null` for `structural_backstop` (no row matched by definition). | +| `kind` | `"affected"`, `"config_rule"`, `"new"`, or `"stranded"`. | +| `reasons` | Empty for `"new"` and `"stranded"`. For `"config_rule"`, names the triggering input path(s). Sorted `(file, kind, collect_sha)`. | +| `reasons[].kind` | `"line_overlap"`, `"structural_backstop"`, `"crate_root_sentinel"`, or `"config_rule"`. | +| `reasons[].stored_range` | `null` for `structural_backstop` and `config_rule` (no coverage row matched). | +| `reasons[].collect_sha` / `matched_hunk` | Empty / `[0, 0]` for `config_rule` (not anchored to a coverage hunk). | ## Stderr summary line diff --git a/src/collect.rs b/src/collect.rs index 0913210..6132097 100644 --- a/src/collect.rs +++ b/src/collect.rs @@ -207,6 +207,7 @@ pub fn collect( Some(&rustflags), Some(&build_dir), &cargo_build_args(nextest_args), + None, )?; eprintln!( "found {} tests across {} binaries", @@ -917,11 +918,16 @@ pub(crate) struct BinaryEntry { /// the build config of the subsequent `cargo nextest run`, or the listing /// enumerates a different test set than the run builds and new-test /// detection ("listed minus DB") becomes unsound. +/// +/// `filter_expr`, when set, passes `-E ` so the listing is restricted to +/// tests matching a nextest filterset — used to resolve `[workspace.metadata.affected]` +/// rules to concrete tests. Leave `None` for a full listing. pub(crate) fn nextest_list( project_root: &Path, rustflags_override: Option<&str>, build_dir: Option<&Path>, build_args: &[String], + filter_expr: Option<&str>, ) -> Result { let mut cmd = Command::new("cargo"); cmd.arg("nextest") @@ -941,6 +947,9 @@ pub(crate) fn nextest_list( for a in build_args { cmd.arg(a); } + if let Some(expr) = filter_expr { + cmd.arg("-E").arg(expr); + } let output = cmd .spawn() .context("failed to spawn cargo nextest list")? diff --git a/src/config.rs b/src/config.rs new file mode 100644 index 0000000..677fee8 --- /dev/null +++ b/src/config.rs @@ -0,0 +1,286 @@ +//! Declarative input→test rules from `[workspace.metadata.affected]`. +//! +//! cargo-affected selects tests by Rust-line coverage overlap, so a change to a +//! non-Rust input a test reads at runtime — an insta `.snap`, a doc `.md`, a +//! template, an `include_str!` target — maps to no coverage row and selects no +//! test (see README, "Non-Rust sources"). The `[[workspace.metadata.affected.rule]]` +//! tables (or `[[package.metadata.affected.rule]]` for a single-crate project) +//! pair input globs with a nextest filterset; when a changed path matches a +//! rule's globs, that rule's tests are force-selected. The filterset is resolved +//! 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. + +use std::collections::{BTreeMap, BTreeSet}; +use std::path::Path; + +use anyhow::{Context, Result}; +use globset::{Glob, GlobSet, GlobSetBuilder}; +use serde::Deserialize; +use serde_json::Value; + +use crate::collect::nextest_list; +use crate::db::TestId; +use crate::project::ProjectRoot; +use crate::selection::{changed_paths_since, ChangedRangesBySha, Reachability}; + +/// Where rules live, for user-facing error messages. The `*` shorthand covers +/// both locations — `[workspace.metadata.affected]` and the single-crate +/// `[package.metadata.affected]` fallback — since these messages fire equally +/// for rules loaded from either, and a hardcoded `workspace` would send a +/// single-crate user grepping for a table their `Cargo.toml` doesn't contain. +const TABLE: &str = "[*.metadata.affected]"; + +/// Parsed `affected` metadata table. An absent table deserializes to `Default` +/// (no rules), preserving the no-config invariant. +#[derive(Debug, Default, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct AffectedConfig { + /// `[[..metadata.affected.rule]]` array-of-tables. Renamed so the TOML key + /// reads as a single rule per table while the field stays plural. + #[serde(default, rename = "rule")] + rules: Vec, +} + +/// One `[[rule]]`: input globs paired with the nextest filterset whose tests +/// to run when any changed path matches. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct InputRule { + globs: Vec, + filterset: String, +} + +/// A rule whose globs are compiled into a matcher, ready to test changed paths. +#[derive(Debug)] +pub(crate) struct CompiledRule { + matcher: GlobSet, + filterset: String, +} + +impl AffectedConfig { + /// Read the `affected` table from the already-loaded `cargo metadata` JSON. + /// + /// Prefers `[workspace.metadata.affected]`; falls back to the root package's + /// `[package.metadata.affected]` (single-crate projects have no `[workspace]` + /// section). An absent table yields no rules — not an error. A malformed + /// table is a hard error: a silently-dropped rule would reopen the exact gap + /// it exists to close. + pub(crate) fn from_metadata(metadata: &Value, workspace_root: &Path) -> Result { + let table = metadata + .get("metadata") + .and_then(|m| m.get("affected")) + .or_else(|| root_package_metadata_affected(metadata, workspace_root)); + match table { + Some(value) => serde_json::from_value(value.clone()) + .with_context(|| format!("failed to parse {TABLE} in Cargo.toml")), + None => Ok(Self::default()), + } + } + + /// Compile every rule's globs into a `GlobSet`. A malformed glob is a hard + /// error naming the offending pattern. + pub(crate) fn compile(&self) -> Result> { + self.rules.iter().map(InputRule::compile).collect() + } +} + +/// The `metadata.affected` value of the package whose manifest is the workspace +/// root's `Cargo.toml` — i.e. the root package's `[package.metadata.affected]`. +fn root_package_metadata_affected<'a>(metadata: &'a Value, workspace_root: &Path) -> Option<&'a Value> { + let root_manifest = workspace_root.join("Cargo.toml"); + metadata.get("packages")?.as_array()?.iter().find_map(|pkg| { + let manifest = pkg.get("manifest_path")?.as_str()?; + if Path::new(manifest) == root_manifest { + pkg.get("metadata")?.get("affected") + } else { + None + } + }) +} + +impl InputRule { + fn compile(&self) -> Result { + let mut builder = GlobSetBuilder::new(); + for g in &self.globs { + builder.add(Glob::new(g).with_context(|| { + format!("invalid glob {g:?} in {TABLE}") + })?); + } + Ok(CompiledRule { + matcher: builder + .build() + .with_context(|| format!("failed to build glob matcher for {TABLE}"))?, + filterset: self.filterset.clone(), + }) + } +} + +/// Load the `affected` rules for `project`, and if any, resolve them against the +/// paths changed since any reachable `collect_sha` (plus working-tree changes). +/// Returns the path→tests map [`compute`](crate::selection::compute) folds in. +/// +/// The no-rules fast path returns an empty map without computing changed paths +/// or invoking nextest, so a project without the table pays nothing. +pub(crate) fn config_rule_hits( + project: &ProjectRoot, + build_args: &[String], + reach: &Reachability, + changed_ranges_by_sha: &ChangedRangesBySha, + working_tree_files: &[String], +) -> Result>> { + let rules = AffectedConfig::from_metadata(&project.metadata, &project.workspace_root)?.compile()?; + if rules.is_empty() { + return Ok(BTreeMap::new()); + } + let project_root = &project.workspace_root; + let changed_paths = + changed_paths_since(project_root, reach, changed_ranges_by_sha, working_tree_files)?; + resolve_config_hits(project_root, build_args, &rules, &changed_paths) +} + +/// Resolve compiled rules against the changed paths, returning a map from each +/// changed path that matched a rule to the tests that rule selects. +/// +/// For each rule with at least one matching changed path, `cargo nextest list +/// -E ` resolves the filterset to concrete tests — using the same +/// build flags as the run, so the listing matches what nextest will build. +/// 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 +/// behavior plus one cheap glob check per changed path. +/// +/// A rule whose filterset resolves to zero tests after matching is surfaced as +/// a warning rather than swallowed: a typo'd filterset would otherwise silently +/// reopen the gap it exists to close. +pub(crate) fn resolve_config_hits( + project_root: &Path, + build_args: &[String], + rules: &[CompiledRule], + changed_paths: &BTreeSet, +) -> Result>> { + let mut out: BTreeMap> = BTreeMap::new(); + for rule in rules { + let matched: Vec<&String> = changed_paths + .iter() + .filter(|p| rule.matcher.is_match(p.as_str())) + .collect(); + if matched.is_empty() { + continue; + } + let listing = nextest_list(project_root, None, None, build_args, Some(&rule.filterset)) + .with_context(|| { + format!( + "failed to resolve {TABLE} filterset {:?} \ + (check it is a valid nextest filter expression)", + rule.filterset + ) + })?; + let tests: BTreeSet = listing.tests.into_iter().collect(); + if tests.is_empty() { + eprintln!( + "warning: {TABLE} rule matched {} but its filterset ({:?}) \ + selected no tests — those input changes may go untested", + matched.iter().map(|s| s.as_str()).collect::>().join(", "), + rule.filterset, + ); + continue; + } + for p in matched { + out.entry(p.clone()).or_default().extend(tests.iter().cloned()); + } + } + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `[workspace.metadata.affected]` shape, as `cargo metadata` surfaces it. + fn workspace_meta(rules: Value) -> Value { + serde_json::json!({ "metadata": { "affected": { "rule": rules } }, "packages": [] }) + } + + #[test] + fn absent_table_is_empty_config() { + let meta = serde_json::json!({ "metadata": null, "packages": [] }); + let cfg = AffectedConfig::from_metadata(&meta, Path::new("/ws")).unwrap(); + assert!(cfg.rules.is_empty()); + assert!(cfg.compile().unwrap().is_empty()); + } + + #[test] + fn parses_workspace_metadata_rules() { + let meta = workspace_meta(serde_json::json!([ + { "globs": ["**/*.snap"], "filterset": "binary_id(=pkg::integration) & test(/help/)" }, + { "globs": ["README.md", "docs/**/*.md"], "filterset": "test(/sync/)" }, + ])); + let cfg = AffectedConfig::from_metadata(&meta, Path::new("/ws")).unwrap(); + assert_eq!(cfg.rules.len(), 2); + assert_eq!(cfg.rules[0].filterset, "binary_id(=pkg::integration) & test(/help/)"); + assert_eq!(cfg.rules[1].globs, vec!["README.md", "docs/**/*.md"]); + } + + #[test] + fn falls_back_to_root_package_metadata() { + // No [workspace.metadata]; rules live in the root package's + // [package.metadata.affected] (single-crate layout). + let meta = serde_json::json!({ + "metadata": null, + "packages": [{ + "manifest_path": "/ws/Cargo.toml", + "metadata": { "affected": { "rule": [ + { "globs": ["x.snap"], "filterset": "test(=t)" } + ] } }, + }], + }); + let cfg = AffectedConfig::from_metadata(&meta, Path::new("/ws")).unwrap(); + assert_eq!(cfg.rules.len(), 1); + assert_eq!(cfg.rules[0].globs, vec!["x.snap"]); + } + + #[test] + fn malformed_table_errors() { + // `filterset` misspelled — deny_unknown_fields makes this a hard error + // rather than a silently-empty rule. + let meta = workspace_meta(serde_json::json!([ + { "globs": ["x"], "filterse": "y" } + ])); + let err = AffectedConfig::from_metadata(&meta, Path::new("/ws")) + .expect_err("typo'd key must error"); + assert!(format!("{err:#}").contains("Cargo.toml")); + } + + #[test] + fn invalid_glob_errors() { + let cfg = AffectedConfig { + rules: vec![InputRule { + globs: vec!["a[".to_string()], // unclosed character class + filterset: "test(/x/)".to_string(), + }], + }; + let err = cfg.compile().expect_err("invalid glob must error"); + assert!(format!("{err:#}").contains("invalid glob")); + } + + #[test] + fn compiled_rule_matches_globs() { + let cfg = AffectedConfig { + rules: vec![InputRule { + globs: vec!["**/*.snap".to_string(), "docs/**/*.md".to_string()], + filterset: "test(/x/)".to_string(), + }], + }; + let compiled = cfg.compile().unwrap(); + let m = &compiled[0].matcher; + assert!(m.is_match("tests/integration/snapshots/help.snap")); + assert!(m.is_match("docs/content/faq.md")); + assert!(!m.is_match("src/lib.rs")); + assert!(!m.is_match("README.md")); + } +} diff --git a/src/db.rs b/src/db.rs index 025657b..8741a27 100644 --- a/src/db.rs +++ b/src/db.rs @@ -188,6 +188,11 @@ pub enum HitKind { /// this file selects this test" link the function-level coverage /// can't observe directly. CrateRootSentinel, + /// The test was force-selected by a `[workspace.metadata.affected]` rule whose + /// globs matched a changed (typically non-Rust) input the coverage model + /// can't link to a test — an insta `.snap`, a doc, a template. Produced by + /// [`crate::config::resolve_config_hits`], not by coverage overlap. + ConfigRule, } /// How many distinct fingerprints to retain. `gc()` evicts the least-recently- diff --git a/src/fingerprint.rs b/src/fingerprint.rs index b6123ab..2fdec6d 100644 --- a/src/fingerprint.rs +++ b/src/fingerprint.rs @@ -6,6 +6,12 @@ //! mapping so queries scoped to the current fingerprint naturally miss when //! any tracked input has changed — no explicit invalidation path needed. //! +//! `[package.metadata]` / `[workspace.metadata]` tables are excluded from the +//! manifest hash (see [`read_manifest_for_fingerprint`]): cargo never reads +//! them for the build, so they can't affect coverage — and one of them, +//! `[workspace.metadata.affected]`, holds this tool's own input rules, which +//! must be editable without invalidating the cache. +//! //! [`compute`] returns both the composite hex digest (for cache scoping) and //! per-component hashes (for diagnostic "which input changed?" reporting). //! Inputs are read once into memory and both digests are derived from the @@ -89,7 +95,7 @@ fn collect_inputs(project: &ProjectRoot) -> Result)>> { .to_string_lossy(); inputs.push(( format!("manifest:{label}"), - read_file_or_empty(manifest)?, + read_manifest_for_fingerprint(manifest)?, )); } @@ -137,6 +143,48 @@ fn read_file_or_empty(path: &std::path::Path) -> Result> { } } +/// Read a manifest's fingerprint bytes with any `[package.metadata]` / +/// `[workspace.metadata]` table stripped. +/// +/// `[*.metadata]` is cargo's escape hatch for external tools; cargo never reads +/// it for the build, so it can't change which code compiles or which tests run +/// — and therefore must not invalidate cached coverage. Concretely, this tool's +/// own `[workspace.metadata.affected]` rules live here: without stripping, +/// editing a rule would change the manifest hash and force a needless +/// re-collect, making config iteration painful. +/// +/// A manifest *without* metadata is hashed by its raw bytes (the round-trip is +/// skipped), so the overwhelming common case is byte-identical to before — no +/// churn on upgrade. Only manifests that carry metadata are re-serialized via +/// toml_edit (which preserves formatting), and only the metadata subtree is +/// removed; editing rules within it leaves the remaining content identical, so +/// the hash is stable across edits. +fn read_manifest_for_fingerprint(path: &std::path::Path) -> Result> { + let raw = read_file_or_empty(path)?; + let Ok(text) = std::str::from_utf8(&raw) else { + // Non-UTF8 isn't valid TOML; hash the raw bytes. + return Ok(raw); + }; + let Ok(mut doc) = text.parse::() else { + // Unparseable manifest: `cargo metadata` would already have failed + // upstream. Hash raw so a broken manifest still differs from a fixed one. + return Ok(raw); + }; + let mut stripped = false; + for section in ["package", "workspace"] { + if let Some(table) = doc.get_mut(section).and_then(|i| i.as_table_like_mut()) { + stripped |= table.remove("metadata").is_some(); + } + } + // Only pay the re-serialize when something was actually removed; otherwise + // the raw bytes hash identically and avoid any dependence on the serializer. + if stripped { + Ok(doc.to_string().into_bytes()) + } else { + Ok(raw) + } +} + fn env_var(name: &str) -> String { std::env::var(name).unwrap_or_default() } @@ -243,6 +291,43 @@ mod tests { Ok(()) } + /// `[*.metadata]` is excluded from the manifest hash: adding it, and then + /// editing it, must not change the fingerprint — otherwise iterating on + /// `[workspace.metadata.affected]` rules would invalidate the coverage cache + /// on every edit. + #[test] + fn manifest_metadata_excluded_from_fingerprint() -> Result<()> { + let _guard = env_lock(); + let dir = tempfile::tempdir()?; + let root = dir.path().to_path_buf(); + let manifest = root.join("Cargo.toml"); + let project = project_with(root.clone(), vec![manifest.clone()]); + + let bare = "[package]\nname = \"a\"\nedition = \"2021\"\n"; + std::fs::write(&manifest, bare)?; + let a = compute(&project)?; + + // Add a rule, then change it. Neither may move the fingerprint. + std::fs::write( + &manifest, + format!("{bare}\n[[package.metadata.affected.rule]]\nglobs = [\"x.snap\"]\nfilterset = \"test(=t)\"\n"), + )?; + let with_rule = compute(&project)?; + std::fs::write( + &manifest, + format!("{bare}\n[[package.metadata.affected.rule]]\nglobs = [\"y.snap\", \"docs/**\"]\nfilterset = \"test(=u)\"\n"), + )?; + let edited = compute(&project)?; + + assert_eq!(with_rule.hex, edited.hex, "editing a rule must not change the fingerprint"); + // First-add may differ by at most the metadata text; the stable + // guarantee is edit-invariance above. Build inputs still register: + std::fs::write(&manifest, format!("{bare}edition2 = true\n"))?; + let real_change = compute(&project)?; + assert_ne!(a.hex, real_change.hex, "a non-metadata manifest edit must still register"); + Ok(()) + } + #[test] fn rustflags_change_changes_hex() -> Result<()> { let _guard = env_lock(); diff --git a/src/main.rs b/src/main.rs index cdce90a..2c0cff9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ //! then queries git for changed files to select which tests to rerun. mod collect; +mod config; mod coverage; mod db; mod fingerprint; diff --git a/src/report.rs b/src/report.rs index f0da29e..885a7c3 100644 --- a/src/report.rs +++ b/src/report.rs @@ -165,6 +165,9 @@ pub struct SelectionReport { pub struct SelectionSummary { pub selected: Option, pub affected: Option, + /// Reachable-known tests force-selected by a `[workspace.metadata.affected]` rule + /// (would otherwise have been skipped). Null on full-suite paths. + pub config: Option, pub new: Option, pub stranded: Option, pub skipped: Option, @@ -208,13 +211,16 @@ pub struct HunkEntry { pub end: i64, } -/// Per-file counts deduplicated by strongest reason — the three values +/// Per-file counts deduplicated by strongest reason — the four values /// sum to [`ChangedFileEntry::tests_pulled_total`]. #[derive(Debug, Serialize, Default)] pub struct ReasonCounts { pub line_overlap: usize, pub structural_backstop: usize, pub crate_root_sentinel: usize, + /// Tests pulled in by a `[workspace.metadata.affected]` rule matching this path. + /// Non-zero only for the (typically non-Rust) inputs such rules target. + pub config_rule: usize, } #[derive(Debug, Serialize)] @@ -233,6 +239,9 @@ pub struct SelectedTestEntry { #[serde(rename_all = "snake_case")] pub enum SelectedTestKind { Affected, + /// Force-selected by a `[workspace.metadata.affected]` rule (reachable-known, would + /// otherwise have been skipped). The `reasons` name the triggering inputs. + ConfigRule, New, Stranded, } @@ -256,6 +265,10 @@ pub enum ReasonKind { LineOverlap, StructuralBackstop, CrateRootSentinel, + /// A `[workspace.metadata.affected]` rule matched the (typically non-Rust) input + /// named in `file`; `collect_sha` is empty and `matched_hunk` is `[0, 0]` + /// since the selection isn't anchored to a coverage hunk. + ConfigRule, } impl From for ReasonKind { @@ -264,6 +277,7 @@ impl From for ReasonKind { HitKind::LineOverlap => Self::LineOverlap, HitKind::StructuralBackstop => Self::StructuralBackstop, HitKind::CrateRootSentinel => Self::CrateRootSentinel, + HitKind::ConfigRule => Self::ConfigRule, } } } @@ -463,6 +477,7 @@ impl Report { let summary = SelectionSummary { selected: Some(selection.selected().len()), affected: Some(selection.affected.len()), + config: Some(selection.config_tests.len()), new: Some(selection.new_tests.len()), stranded: Some(selection.stranded_tests.len()), skipped: Some(selection.skipped()), @@ -482,6 +497,7 @@ impl Report { let selected_tests = selection.diagnostics.per_test.as_ref().map(|per_test| { build_selected_tests( &selection.affected, + &selection.config_tests, &selection.new_tests, &selection.stranded_tests, per_test, @@ -528,6 +544,7 @@ impl Report { summary: SelectionSummary { selected: None, affected: None, + config: None, new: None, stranded: None, skipped: None, @@ -731,6 +748,7 @@ fn build_changed_files_entries( line_overlap: counts.line_overlap, structural_backstop: counts.structural_backstop, crate_root_sentinel: counts.crate_root_sentinel, + config_rule: counts.config_rule, }, } }) @@ -745,14 +763,17 @@ fn build_changed_files_entries( fn build_selected_tests( affected: &BTreeSet, + config_tests: &BTreeSet, new_tests: &BTreeSet, stranded: &BTreeSet, per_test: &BTreeMap>, ) -> Vec { - // Union all selected tests, classify, and emit in a stable order. + // Union all selected tests, classify, and emit in a stable order. The four + // sets are disjoint, so the classification order only needs to be exhaustive. let mut out: Vec = Vec::new(); let union: BTreeSet<&TestId> = affected .iter() + .chain(config_tests.iter()) .chain(new_tests.iter()) .chain(stranded.iter()) .collect(); @@ -761,6 +782,8 @@ fn build_selected_tests( SelectedTestKind::New } else if stranded.contains(test) { SelectedTestKind::Stranded + } else if config_tests.contains(test) { + SelectedTestKind::ConfigRule } else { SelectedTestKind::Affected }; diff --git a/src/run.rs b/src/run.rs index c294ce2..581633b 100644 --- a/src/run.rs +++ b/src/run.rs @@ -31,6 +31,7 @@ use crate::collect::{ cargo_build_args, nextest_filter_expr, nextest_list, require_nextest, write_nextest_config, }; +use crate::config; use crate::db::{warn_untracked_rs_files, Db, TestId}; use crate::fingerprint::{self, Fingerprint}; use crate::project::{find_project_root, git_changed_files, ShaRelation}; @@ -184,18 +185,30 @@ pub fn run( // List with the same cargo build flags `run_tests` hands to `nextest // run`, so new-test detection compares against the test set the run // actually builds — not a feature-less one. - let listing = nextest_list(project_root, None, None, &cargo_build_args(nextest_args))?; + let build_args = cargo_build_args(nextest_args); + let listing = nextest_list(project_root, None, None, &build_args, None)?; // Compute per-sha hunks once; selection consumes them and so does // the report builder if --report-json is set. The previous code // ran `git diff -U0 ` twice per reachable sha on the report // path. let changed_ranges = selection::changed_ranges_per_sha(project_root, &reach.reachable)?; + // Declarative input rules ([workspace.metadata.affected]): force-select tests whose + // non-Rust inputs (snapshots, docs, templates) changed — coverage can't + // link those to tests. No config file → no rules → zero extra work. + let config_hits = config::config_rule_hits( + &project, + &build_args, + &reach, + &changed_ranges, + &changed_files, + )?; let sel = selection::select_with_precomputed_ranges( &db, &fingerprint.hex, &listing, &reach, &changed_ranges, + &config_hits, detail, )?; diff --git a/src/selection.rs b/src/selection.rs index ccb5376..d7ed7dd 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -22,7 +22,9 @@ use anyhow::Result; use crate::collect::Listing; use crate::db::{Db, HitKind, HitReason, TestId}; -use crate::project::{git_changed_line_ranges, relation_to_head, LineRange, ShaRelation}; +use crate::project::{ + git_added_files_since, git_changed_line_ranges, relation_to_head, LineRange, ShaRelation, +}; /// Result of the selection computation. pub(crate) struct Selection { @@ -43,6 +45,15 @@ pub(crate) struct Selection { /// them so consumers can tell the difference between "added in this /// PR" and "anchor sha got rebased away". pub(crate) stranded_tests: BTreeSet, + /// Reachable-known tests force-selected by a `[workspace.metadata.affected]` rule — + /// a changed input (snapshot, doc, template) matched a rule's globs and + /// coverage couldn't link it to the test. Disjoint from [`affected`] + /// (a test pulled in by both counts as `affected`) and from + /// `new`/`stranded` (those aren't reachable-known); these would have been + /// *skipped* without the rule. Excludes `#[ignore]`d tests. + /// + /// [`affected`]: Self::affected + pub(crate) config_tests: BTreeSet, /// Distinct test count tracked under the current fingerprint at /// reachable shas. The "tests we could have selected from" denominator. pub(crate) reachable_known_count: usize, @@ -56,19 +67,21 @@ pub(crate) struct Selection { } impl Selection { - /// Union of affected, stranded, and new tests — what nextest will be - /// asked to run. + /// Union of affected, stranded, new, and config-rule tests — what nextest + /// will be asked to run. pub(crate) fn selected(&self) -> BTreeSet { let mut out = self.affected.clone(); out.extend(self.new_tests.iter().cloned()); out.extend(self.stranded_tests.iter().cloned()); + out.extend(self.config_tests.iter().cloned()); out } - /// Known tests not selected this round. + /// Known tests not selected this round. Both `affected` and `config_tests` + /// are reachable-known and selected, so both reduce the skipped count. pub(crate) fn skipped(&self) -> usize { self.reachable_known_count - .saturating_sub(self.affected.len()) + .saturating_sub(self.affected.len() + self.config_tests.len()) } } @@ -98,25 +111,29 @@ pub(crate) struct SelectionDiagnostics { /// Counts are deduplicated by strongest reason: a test with both a /// LineOverlap hit and a CrateRootSentinel hit on the same file counts /// once, classified by the strongest reason -/// (LineOverlap > StructuralBackstop > CrateRootSentinel). Per-file -/// counts therefore sum to `total_unique_tests`, making the diagnostic -/// arithmetic clean. +/// (LineOverlap > StructuralBackstop > ConfigRule > CrateRootSentinel). +/// Per-file counts therefore sum to `total_unique_tests`, making the +/// diagnostic arithmetic clean. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub(crate) struct FileReasonCounts { pub(crate) line_overlap: usize, pub(crate) structural_backstop: usize, pub(crate) crate_root_sentinel: usize, + pub(crate) config_rule: usize, pub(crate) total_unique_tests: usize, } /// Strongest-reason ordering. Used to dedupe per-test reasons when /// rolling up to per-file counts: a test counts ONCE per file, by its -/// strongest reason. +/// strongest reason. ConfigRule ranks below the coverage-derived reasons +/// (its file has no coverage rows, so in practice it never co-occurs with +/// them on the same file) but above the bare sentinel. fn strongest(a: HitKind, b: HitKind) -> HitKind { fn rank(k: HitKind) -> u8 { match k { - HitKind::LineOverlap => 2, - HitKind::StructuralBackstop => 1, + HitKind::LineOverlap => 3, + HitKind::StructuralBackstop => 2, + HitKind::ConfigRule => 1, HitKind::CrateRootSentinel => 0, } } @@ -241,12 +258,16 @@ pub(crate) fn select_with_reach( detail: DiagnosticDetail, ) -> Result { let changed_ranges_by_sha = changed_ranges_per_sha(project_root, &reach.reachable)?; + // `collect --diff` recollects coverage for changed Rust code; config rules + // select tests to *run* despite absent coverage, which is a `run`/`status` + // concern. Pass no config hits here. compute( db, fingerprint, &reach.reachable, &changed_ranges_by_sha, listing, + &BTreeMap::new(), detail, ) } @@ -262,6 +283,7 @@ pub(crate) fn select_with_precomputed_ranges( listing: &Listing, reach: &Reachability, changed_ranges_by_sha: &ChangedRangesBySha, + config_hits: &BTreeMap>, detail: DiagnosticDetail, ) -> Result { compute( @@ -270,10 +292,36 @@ pub(crate) fn select_with_precomputed_ranges( &reach.reachable, changed_ranges_by_sha, listing, + config_hits, detail, ) } +/// Union of all paths that changed between the working tree and any reachable +/// `collect_sha` — for matching against `[workspace.metadata.affected]` rule globs. +/// +/// Modified files come from the per-sha diff already computed for selection; +/// added files (which `git diff -U0` omits — they have no OLD side) come from +/// [`git_added_files_since`]; working-tree changes (uncommitted, staged, +/// untracked) come from `working_tree_files`. Without the added-files source, +/// a PR that adds a brand-new `.snap`/doc with no modified sibling would slip +/// through. +pub(crate) fn changed_paths_since( + project_root: &Path, + reach: &Reachability, + changed_ranges_by_sha: &ChangedRangesBySha, + working_tree_files: &[String], +) -> Result> { + let mut paths: BTreeSet = working_tree_files.iter().cloned().collect(); + for by_file in changed_ranges_by_sha.values() { + paths.extend(by_file.keys().cloned()); + } + for sha in &reach.reachable { + paths.extend(git_added_files_since(project_root, sha)?); + } + Ok(paths) +} + /// Compute the selection from a pre-built nextest listing and per-sha changed /// ranges. Most callers should use [`select_with_reach`] instead; this is the /// lower-level entry point for tests and any caller that already has the @@ -305,6 +353,7 @@ pub(crate) fn compute( reachable_shas: &BTreeSet, changed_ranges_by_sha: &ChangedRangesBySha, listing: &Listing, + config_hits: &BTreeMap>, detail: DiagnosticDetail, ) -> Result { // Mark this fingerprint as recently used so the next collect's LRU @@ -384,6 +433,41 @@ pub(crate) fn compute( } } + // Declarative input rules: a changed (typically non-Rust) input matched a + // `[workspace.metadata.affected]` rule. Coverage can't link these inputs to tests, so + // the rule supplies the edge. A reachable-known test that isn't already + // `affected` would otherwise be skipped — rescue it as a `config_test`. + // New/stranded matches already run; ignored ones stay skipped by nextest. + let mut config_tests = BTreeSet::new(); + for (path, tests) in config_hits { + for test in tests { + if listing.ignored.contains(test) + || affected.contains(test) + || !reachable_known.contains(test) + { + continue; + } + config_tests.insert(test.clone()); + strongest_per_file_test + .entry(path.clone()) + .or_default() + .entry(test.clone()) + .and_modify(|k| *k = strongest(*k, HitKind::ConfigRule)) + .or_insert(HitKind::ConfigRule); + if retain_per_test { + // Config reasons name the triggering input path; they have no + // sha-anchored coverage hunk, so those fields are left empty. + per_test_reasons.entry(test.clone()).or_default().push(HitReason { + collect_sha: String::new(), + file: path.clone(), + kind: HitKind::ConfigRule, + matched_hunk: (0, 0), + stored_range: None, + }); + } + } + } + let per_file = aggregate_per_file_counts(&strongest_per_file_test); let diagnostics = SelectionDiagnostics { per_file, @@ -394,6 +478,7 @@ pub(crate) fn compute( affected, new_tests, stranded_tests, + config_tests, reachable_known_count, listed, diagnostics, @@ -414,6 +499,7 @@ fn aggregate_per_file_counts( HitKind::LineOverlap => counts.line_overlap += 1, HitKind::StructuralBackstop => counts.structural_backstop += 1, HitKind::CrateRootSentinel => counts.crate_root_sentinel += 1, + HitKind::ConfigRule => counts.config_rule += 1, } counts.total_unique_tests += 1; } @@ -429,10 +515,11 @@ fn aggregate_per_file_counts( pub(crate) fn format_summary(sel: &Selection, verb: &str, verbose: bool) -> String { let selected = sel.selected(); let mut out = format!( - "{} tests {verb} ({} affected + {} new + {} stranded, \ + "{} tests {verb} ({} affected + {} config + {} new + {} stranded, \ {} skipped of {} reachable-known)", selected.len(), sel.affected.len(), + sel.config_tests.len(), sel.new_tests.len(), sel.stranded_tests.len(), sel.skipped(), @@ -445,6 +532,8 @@ pub(crate) fn format_summary(sel: &Selection, verb: &str, verbose: bool) -> Stri " (new)" } else if sel.stranded_tests.contains(t) { " (stranded)" + } else if sel.config_tests.contains(t) { + " (config)" } else { "" }; @@ -468,6 +557,7 @@ mod tests { affected: &[TestId], new_tests: &[TestId], stranded_tests: &[TestId], + config_tests: &[TestId], reachable_known_count: usize, ) -> Selection { let listed: BTreeSet = affected @@ -475,11 +565,13 @@ mod tests { .cloned() .chain(new_tests.iter().cloned()) .chain(stranded_tests.iter().cloned()) + .chain(config_tests.iter().cloned()) .collect(); Selection { affected: affected.iter().cloned().collect(), new_tests: new_tests.iter().cloned().collect(), stranded_tests: stranded_tests.iter().cloned().collect(), + config_tests: config_tests.iter().cloned().collect(), reachable_known_count, listed, diagnostics: SelectionDiagnostics { @@ -495,42 +587,48 @@ mod tests { &[tid("crate_a", "test_a"), tid("crate_a", "test_b")], &[tid("crate_a", "test_c")], &[], + &[], 5, ); let out = format_summary(&sel, "to run", false); assert_eq!( out, - "3 tests to run (2 affected + 1 new + 0 stranded, \ + "3 tests to run (2 affected + 0 config + 1 new + 0 stranded, \ 3 skipped of 5 reachable-known) — pass -v to list" ); } #[test] - fn summary_verbose_tags_new_and_stranded() { + fn summary_verbose_tags_categories() { let sel = selection_with( &[tid("crate_a", "test_a")], &[tid("crate_a", "test_b")], &[tid("crate_a", "test_c")], - 4, + &[tid("crate_a", "test_d")], + 5, ); let out = format_summary(&sel, "would run", true); assert_eq!( out, - "3 tests would run (1 affected + 1 new + 1 stranded, \ - 3 skipped of 4 reachable-known):\n \ + "4 tests would run (1 affected + 1 config + 1 new + 1 stranded, \ + 3 skipped of 5 reachable-known):\n \ crate_a::test_a\n \ crate_a::test_b (new)\n \ - crate_a::test_c (stranded)" + crate_a::test_c (stranded)\n \ + crate_a::test_d (config)" ); } #[test] - fn skipped_saturates_when_all_known_selected() { + fn skipped_subtracts_affected_and_config() { + // 2 affected + 1 config, all reachable-known → all 3 selected, none + // skipped. let sel = selection_with( &[tid("crate_a", "a"), tid("crate_a", "b")], &[], &[], - 2, + &[tid("crate_a", "c")], + 3, ); assert_eq!(sel.skipped(), 0); } diff --git a/src/status.rs b/src/status.rs index bbb5661..d312bd9 100644 --- a/src/status.rs +++ b/src/status.rs @@ -14,6 +14,7 @@ use std::path::Path; use anyhow::Result; use crate::collect::{nextest_list, require_nextest}; +use crate::config; use crate::db::{db_path, warn_untracked_rs_files, Db, StoredFingerprintRow}; use crate::fingerprint::{self, Fingerprint}; use crate::project::{find_project_root, git_changed_files, ShaRelation}; @@ -177,14 +178,24 @@ pub fn status( eprintln!("checking for new tests..."); // `status` takes no passthrough args, so there are no build flags to // thread through — list the default build. - let listing = nextest_list(project_root, None, None, &[])?; + let listing = nextest_list(project_root, None, None, &[], None)?; let changed_ranges = selection::changed_ranges_per_sha(project_root, &reach.reachable)?; + // Mirror `run`: apply [workspace.metadata.affected] input rules so the dry-run + // predicts the same selection `run` would make. + let config_hits = config::config_rule_hits( + &project, + &[], + &reach, + &changed_ranges, + &changed_files, + )?; let sel = selection::select_with_precomputed_ranges( &db, &fingerprint.hex, &listing, &reach, &changed_ranges, + &config_hits, detail, )?; diff --git a/tests/functional/config_rule.rs b/tests/functional/config_rule.rs new file mode 100644 index 0000000..4890833 --- /dev/null +++ b/tests/functional/config_rule.rs @@ -0,0 +1,176 @@ +//! `[*.metadata.affected]` input rules: force-select tests whose non-Rust +//! inputs changed. +//! +//! A test that reads a data file at runtime (the insta-snapshot / doc-sync +//! shape) has coverage rows only for the Rust lines it executed — never for the +//! data file. So a change confined to that file overlaps no coverage and the +//! test is *skipped*, even though it would fail. This is the documented +//! non-Rust-input false-negative. A `[[package.metadata.affected.rule]]` mapping +//! the file's glob to the test closes the gap. We assert both halves: the miss +//! without a rule, the rescue with it. (Metadata is excluded from the +//! fingerprint, so adding the rule after `collect` keeps the same cache.) + +use std::path::Path; + +use crate::{ + cargo_affected, combined_output, git, init_git_with_initial_commit, replace_in_file, +}; + +/// Crate whose only test reads `golden.txt` at runtime and compares it to a +/// `const` — a hermetic stand-in for an insta snapshot or doc-sync test. +fn write_golden_project(dir: &Path) { + std::fs::write( + dir.join("Cargo.toml"), + r#"[package] +name = "config-rule-sample" +version = "0.1.0" +edition = "2021" +"#, + ) + .unwrap(); + std::fs::write(dir.join(".gitignore"), "/target\n/Cargo.lock\n").unwrap(); + + let src = dir.join("src"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write(src.join("lib.rs"), "pub const GREETING: &str = \"hello\";\n").unwrap(); + + // The data file the test reads at runtime. llvm-cov never sees it, so no + // coverage row links it to `golden_matches`. + std::fs::write(dir.join("golden.txt"), "hello\n").unwrap(); + + let tests = dir.join("tests"); + std::fs::create_dir_all(&tests).unwrap(); + std::fs::write( + tests.join("golden.rs"), + r#"#[test] +fn golden_matches() { + let expected = std::fs::read_to_string( + concat!(env!("CARGO_MANIFEST_DIR"), "/golden.txt"), + ) + .unwrap(); + assert_eq!(config_rule_sample::GREETING, expected.trim()); +} +"#, + ) + .unwrap(); +} + +/// Append a `[[package.metadata.affected.rule]]` to the sample crate's +/// Cargo.toml. `globs` is the TOML array body (e.g. `"golden.txt"`). +fn add_affected_rule(dir: &Path, globs: &str, filterset: &str) { + let cargo_toml = dir.join("Cargo.toml"); + let mut content = std::fs::read_to_string(&cargo_toml).unwrap(); + content.push_str(&format!( + "\n[[package.metadata.affected.rule]]\nglobs = [{globs}]\nfilterset = \"{filterset}\"\n" + )); + std::fs::write(&cargo_toml, content).unwrap(); +} + +#[test] +fn config_rule_selects_test_for_non_rust_input_change() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + write_golden_project(dir); + init_git_with_initial_commit(dir); + + // Seed coverage: `golden_matches` runs, covering `GREETING` and the test + // body — but nothing links `golden.txt` to it. + let collect = cargo_affected(dir, &["affected", "collect"]); + assert!(collect.status.success(), "collect failed: {}", combined_output(&collect)); + + // The non-Rust input changes. No Rust hunk → coverage selects nothing. + replace_in_file(&dir.join("golden.txt"), "hello", "hi"); + + // --- Without a rule: the change is a coverage blind spot (the miss). --- + let miss = combined_output(&cargo_affected(dir, &["affected", "status", "-v"])); + assert!( + miss.contains("selection=0/1"), + "expected the golden test skipped (0 of 1 selected): {miss}" + ); + assert!( + !miss.contains("golden_matches"), + "golden_matches should NOT be selected without a rule: {miss}" + ); + + // --- Add the rule (metadata isn't fingerprinted, so the cache survives). --- + add_affected_rule(dir, "\"golden.txt\"", "test(=golden_matches)"); + let fixed = combined_output(&cargo_affected(dir, &["affected", "status", "-v"])); + assert!( + fixed.contains("selection=1/1"), + "expected the golden test selected (1 of 1): {fixed}" + ); + assert!( + fixed.contains("1 config"), + "expected the rescue attributed to the config category: {fixed}" + ); + assert!( + fixed.contains("golden_matches (config)"), + "expected golden_matches tagged (config): {fixed}" + ); + assert!( + fixed.contains("0 skipped of 1 reachable-known"), + "the rescued test should no longer be skipped: {fixed}" + ); +} + +/// A *committed* added input (a new file since `collect_sha`) must rescue its +/// rule's tests. This exercises the `git_added_files_since` path specifically: +/// `git diff -U0` omits a new file (no OLD side) and working-tree queries don't +/// see a committed file, so without that source the addition would be invisible +/// and the gap would silently reopen. +#[test] +fn config_rule_rescues_committed_added_input() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + write_golden_project(dir); + add_affected_rule(dir, "\"data/*.snap\"", "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)); + + // Add a brand-new input and commit it: it's an addition since collect_sha, + // with no modified sibling. Only `git_added_files_since` surfaces it. + std::fs::create_dir_all(dir.join("data")).unwrap(); + std::fs::write(dir.join("data/new.snap"), "x\n").unwrap(); + git(dir, &["add", "data/new.snap"]); + git(dir, &["commit", "-q", "-m", "add snapshot"]); + + let out = combined_output(&cargo_affected(dir, &["affected", "status", "-v"])); + assert!( + out.contains("1 config"), + "a committed added input should rescue the test via config: {out}" + ); + assert!( + out.contains("golden_matches (config)"), + "golden_matches should be config-selected for the added input: {out}" + ); +} + +/// 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] +fn config_rule_inert_when_no_glob_matches() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + write_golden_project(dir); + // A rule keyed on a file the edit below never touches. + 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)); + + // Edit a Rust file (not golden.txt). The rule's glob doesn't match, so the + // config category stays empty and selection is driven purely by coverage. + replace_in_file(&dir.join("src/lib.rs"), "hello", "hello world"); + let out = combined_output(&cargo_affected(dir, &["affected", "status", "-v"])); + assert!( + out.contains("0 config"), + "a non-matching rule must add nothing: {out}" + ); + assert!( + out.contains("golden_matches"), + "the GREETING edit should still select the test via coverage: {out}" + ); +} diff --git a/tests/functional/main.rs b/tests/functional/main.rs index 2821ae9..76ed6b7 100644 --- a/tests/functional/main.rs +++ b/tests/functional/main.rs @@ -23,6 +23,7 @@ mod cache_miss; mod clean; +mod config_rule; mod db_has_function_ranges; mod diff_collect; mod dirty;