Skip to content

Commit 8204255

Browse files
authored
Merge pull request #639 from pulseengine/fix/603-verify-workspace-scan
fix(verify): default marker scan reaches member crates in a workspace (REQ-242, #603)
2 parents 18dd82b + 3546d42 commit 8204255

2 files changed

Lines changed: 94 additions & 27 deletions

File tree

artifacts/requirements.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7612,7 +7612,7 @@ artifacts:
76127612
- id: REQ-242
76137613
type: requirement
76147614
title: rivet verify default scan finds member-crate markers in a mixed workspace
7615-
status: proposed
7615+
status: verified
76167616
description: "In a cargo workspace where the repo root has its OWN src/ or tests/ (not just member crates), `rivet verify`'s default scan misses `verifies` markers inside member crates — under-counting real test evidence. Make the default scan cover member-crate src/tests even when a root crate is present. #603."
76177617
provenance:
76187618
created-by: ai-assisted

rivet-cli/src/main.rs

Lines changed: 93 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7431,6 +7431,38 @@ fn cmd_stats_qualification(cli: &Cli) -> Result<bool> {
74317431
Ok(true)
74327432
}
74337433

7434+
/// Default directories to scan for `// rivet: verifies <ID>` source markers
7435+
/// when the user gives no explicit `--scan`. Shared by `rivet verify` and
7436+
/// `rivet coverage --tests` so the two never disagree on where evidence lives.
7437+
///
7438+
/// In a **cargo workspace** (root `Cargo.toml` has `[workspace]`) this returns
7439+
/// the project root, so the recursive scan reaches member-crate `src/`+`tests/`
7440+
/// (e.g. `rivet-cli/tests/`). `scan_source_files` already skips `target/`,
7441+
/// `node_modules/`, and dot-dirs, so scanning from the root is safe. Otherwise
7442+
/// it returns the root `src/`+`tests/` that exist, falling back to the root.
7443+
///
7444+
/// Before this, the default stopped at root `./src`+`./tests` whenever *either*
7445+
/// existed — so a workspace whose root ALSO has its own `tests/` (rivet itself)
7446+
/// never recursed into member crates, and `rivet verify` reported "no evidence"
7447+
/// for markers that lived in `rivet-cli/tests/` (#603, follow-up to #574).
7448+
fn default_marker_scan_paths(project: &std::path::Path) -> Vec<std::path::PathBuf> {
7449+
let is_workspace = std::fs::read_to_string(project.join("Cargo.toml"))
7450+
.map(|c| c.lines().any(|l| l.trim_start().starts_with("[workspace]")))
7451+
.unwrap_or(false);
7452+
if is_workspace {
7453+
return vec![project.to_path_buf()];
7454+
}
7455+
let mut paths: Vec<std::path::PathBuf> = ["src", "tests"]
7456+
.iter()
7457+
.map(|d| project.join(d))
7458+
.filter(|c| c.is_dir())
7459+
.collect();
7460+
if paths.is_empty() {
7461+
paths.push(project.to_path_buf());
7462+
}
7463+
paths
7464+
}
7465+
74347466
/// #559: advance an artifact to `verified` when it has verifying evidence —
74357467
/// an incoming `verifies` link, OR a `// rivet: verifies <ID>` source marker.
74367468
/// Opt-in and auditable (no auto-advance); the artifact must be `implemented`.
@@ -7459,18 +7491,10 @@ fn cmd_verify(cli: &Cli, id: &str, scan: &[std::path::PathBuf]) -> Result<bool>
74597491
ctx.graph.backlinks_of_type(id, "verifies").len()
74607492
};
74617493

7462-
// Source-marker evidence: `// rivet: verifies <ID>`. Default scan dirs match
7463-
// `coverage --tests`: src/ + tests/, else the project root.
7494+
// Source-marker evidence: `// rivet: verifies <ID>`. Default scan dirs are
7495+
// workspace-aware and shared with `coverage --tests` (#603).
74647496
let paths: Vec<std::path::PathBuf> = if scan.is_empty() {
7465-
let mut p: Vec<std::path::PathBuf> = ["src", "tests"]
7466-
.iter()
7467-
.map(|d| cli.project.join(d))
7468-
.filter(|c| c.is_dir())
7469-
.collect();
7470-
if p.is_empty() {
7471-
p.push(cli.project.clone());
7472-
}
7473-
p
7497+
default_marker_scan_paths(&cli.project)
74747498
} else {
74757499
scan.to_vec()
74767500
};
@@ -8262,22 +8286,10 @@ fn cmd_coverage_tests(cli: &Cli, format: &str, scan_paths: &[PathBuf]) -> Result
82628286
let ctx = ProjectContext::load(cli)?;
82638287
let (store, schema) = (ctx.store, ctx.schema);
82648288

8265-
// Resolve scan paths: default to src/ and tests/ relative to project dir.
8289+
// Resolve scan paths: workspace-aware defaults shared with `rivet verify`
8290+
// so the two agree on where markers live (#603).
82668291
let paths: Vec<PathBuf> = if scan_paths.is_empty() {
8267-
let mut defaults = Vec::new();
8268-
let src = cli.project.join("src");
8269-
let tests = cli.project.join("tests");
8270-
if src.is_dir() {
8271-
defaults.push(src);
8272-
}
8273-
if tests.is_dir() {
8274-
defaults.push(tests);
8275-
}
8276-
// If neither exists, scan the project root.
8277-
if defaults.is_empty() {
8278-
defaults.push(cli.project.clone());
8279-
}
8280-
defaults
8292+
default_marker_scan_paths(&cli.project)
82818293
} else {
82828294
scan_paths
82838295
.iter()
@@ -19364,3 +19376,58 @@ mod export_static_links_tests {
1936419376
assert!(!out.contains("_assets/docs//etc"), "{out}");
1936519377
}
1936619378
}
19379+
19380+
#[cfg(test)]
19381+
mod marker_scan_path_tests {
19382+
use super::default_marker_scan_paths;
19383+
19384+
/// #603: in a cargo workspace whose root ALSO has its own `tests/`
19385+
/// (rivet itself), the default marker scan must reach member crates —
19386+
/// i.e. it returns the project ROOT (recursive), not just root src/tests.
19387+
///
19388+
/// rivet: verifies REQ-242
19389+
#[test]
19390+
fn workspace_with_root_tests_scans_from_root() {
19391+
let tmp = tempfile::tempdir().unwrap();
19392+
let root = tmp.path();
19393+
std::fs::write(
19394+
root.join("Cargo.toml"),
19395+
"[workspace]\nmembers = [\"member\"]\n",
19396+
)
19397+
.unwrap();
19398+
// Root has its own tests/ AND src/ — the exact #603 trigger that used
19399+
// to stop the scan at the root dirs.
19400+
std::fs::create_dir_all(root.join("tests")).unwrap();
19401+
std::fs::create_dir_all(root.join("src")).unwrap();
19402+
std::fs::create_dir_all(root.join("member/tests")).unwrap();
19403+
19404+
let paths = default_marker_scan_paths(root);
19405+
assert_eq!(
19406+
paths,
19407+
vec![root.to_path_buf()],
19408+
"a workspace must scan from the root so member-crate tests are covered"
19409+
);
19410+
}
19411+
19412+
/// A non-workspace project still uses root src/+tests/ (unchanged).
19413+
#[test]
19414+
fn non_workspace_uses_root_src_and_tests() {
19415+
let tmp = tempfile::tempdir().unwrap();
19416+
let root = tmp.path();
19417+
std::fs::write(root.join("Cargo.toml"), "[package]\nname = \"x\"\n").unwrap();
19418+
std::fs::create_dir_all(root.join("src")).unwrap();
19419+
std::fs::create_dir_all(root.join("tests")).unwrap();
19420+
19421+
let paths = default_marker_scan_paths(root);
19422+
assert_eq!(paths, vec![root.join("src"), root.join("tests")]);
19423+
}
19424+
19425+
/// A bare directory (no src/tests, no workspace) falls back to the root.
19426+
#[test]
19427+
fn bare_project_falls_back_to_root() {
19428+
let tmp = tempfile::tempdir().unwrap();
19429+
let root = tmp.path();
19430+
let paths = default_marker_scan_paths(root);
19431+
assert_eq!(paths, vec![root.to_path_buf()]);
19432+
}
19433+
}

0 commit comments

Comments
 (0)