Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9e94891
feat: camelCase-only TestPlan Node API with includeGlob
jonathanong Aug 22, 2026
25762a4
feat: add --profile ci and memoize ciTopology in-process
jonathanong Aug 22, 2026
35a377f
fix: keep Node plan documents round-trippable after camelCase
jonathanong Aug 22, 2026
14d02db
merge: pick up TestPlan camelCase round-trip fixes
jonathanong Aug 22, 2026
ea5f6db
docs: resolve node-api merge conflict markers
jonathanong Aug 22, 2026
965b137
fix: decamelize string planJson and keep testsWhy path keys
jonathanong Aug 22, 2026
7321e5f
merge: TestPlan document round-trip from #764
jonathanong Aug 22, 2026
09c847c
fix: key ciTopology memo by workflows and honor profile ci timeouts
jonathanong Aug 22, 2026
97f12e5
style: oxfmt ciTopology memo helper
jonathanong Aug 22, 2026
433265b
lint: avoid copying Map keys just to iterate them
jonathanong Aug 22, 2026
099a138
fix: round-trip camelCase TestPlan files and batched reports
jonathanong Aug 22, 2026
0975ed2
style: oxfmt analyzeProject camelize dispatch
jonathanong Aug 22, 2026
862eb1b
merge: camelCase plan files and batched reports from #764
jonathanong Aug 22, 2026
dfab19c
fix: keep testsWhy on plan paths instead of planJson
jonathanong Aug 22, 2026
4ad41e9
merge: testsWhy plan-path materialization from #764
jonathanong Aug 22, 2026
ddd7c9e
fix: give each materialized testsWhy plan its own temp directory
jonathanong Aug 23, 2026
02b58c4
merge: unique testsWhy temp dirs from #764
jonathanong Aug 23, 2026
8ea9180
fix: remove generated testsWhy plan directories after native calls
jonathanong Aug 23, 2026
4b707bd
merge: testsWhy generated-dir cleanup from #764
jonathanong Aug 23, 2026
094ca87
fix: declare planJson on TestsWhyOptions
jonathanong Aug 23, 2026
494875d
merge: TestsWhyOptions planJson from #764
jonathanong Aug 23, 2026
4439515
merge: origin/main after #764
jonathanong Aug 23, 2026
2f62730
fix: keep node-api runtime export table parseable
jonathanong Aug 23, 2026
22f1d28
fix: pin native ciTopology calls to the resolved absolute root
jonathanong Aug 23, 2026
f3b3e1a
merge: origin/main after #766
jonathanong Aug 23, 2026
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
1 change: 1 addition & 0 deletions crates/no-mistakes/src/impacted_checks/generate/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub(crate) fn plan_args_for(
format: None,
json: false,
include_comment: false,
include_glob: Vec::new(),
}
}

Expand Down
18 changes: 16 additions & 2 deletions crates/no-mistakes/src/invocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ fn deadline_test_lock() -> &'static std::sync::Mutex<()> {
LOCK.get_or_init(|| std::sync::Mutex::new(()))
}

#[derive(clap::ValueEnum, Debug, Clone, Copy, PartialEq, Eq)]
enum InvocationProfile {
/// Unbounded command and lock wait (`timeout: 0`, `lock-timeout: 0`).
Ci,
}

#[derive(clap::Args, Debug, Clone, Copy)]
pub struct InvocationArgs {
/// Maximum command execution time in seconds; 0 disables the deadline.
Expand All @@ -67,6 +73,9 @@ pub struct InvocationArgs {
/// Fail immediately when another no-mistakes invocation holds the lock.
#[arg(long, global = true)]
fail_on_lock: bool,
/// Named timeout defaults. `ci` sets `--timeout 0 --lock-timeout 0`.
#[arg(long, value_enum, global = true)]
profile: Option<InvocationProfile>,
}

impl Default for InvocationArgs {
Expand All @@ -75,15 +84,20 @@ impl Default for InvocationArgs {
timeout: DEFAULT_TIMEOUT_SECONDS,
lock_timeout: DEFAULT_TIMEOUT_SECONDS,
fail_on_lock: false,
profile: None,
}
}
}

impl InvocationArgs {
pub fn options(self) -> InvocationOptions {
let (timeout, lock_timeout) = match self.profile {
Some(InvocationProfile::Ci) => (0, 0),
None => (self.timeout, self.lock_timeout),
};
InvocationOptions {
timeout: nonzero_seconds(self.timeout),
lock_timeout: nonzero_seconds(self.lock_timeout),
timeout: nonzero_seconds(timeout),
lock_timeout: nonzero_seconds(lock_timeout),
fail_on_lock: self.fail_on_lock,
jobs: None,
}
Expand Down
9 changes: 9 additions & 0 deletions crates/no-mistakes/src/invocation/napi_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ pub fn extract_napi_options_value(
}
};
let jobs = take_jobs(object)?;
match object.remove("profile") {
None | Some(Value::Null) => {}
Some(Value::String(value)) if value == "ci" => {}
Comment thread
jonathanong marked this conversation as resolved.
Outdated
Some(_) => {
return Err(anyhow!(
"invalid options JSON: profile must be \"ci\" when set"
))
}
}
Comment thread
jonathanong marked this conversation as resolved.
Outdated
Ok((
value,
InvocationOptions {
Expand Down
20 changes: 20 additions & 0 deletions crates/no-mistakes/src/invocation/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ fn cli_defaults_and_zero_values_have_napi_parity() {
timeout: 0,
lock_timeout: 0,
fail_on_lock: true,
profile: None,
}
.options(),
InvocationOptions {
Expand All @@ -58,6 +59,25 @@ fn cli_defaults_and_zero_values_have_napi_parity() {
);
}

#[test]
fn ci_profile_disables_timeouts() {
assert_eq!(
InvocationArgs {
timeout: DEFAULT_TIMEOUT_SECONDS,
lock_timeout: DEFAULT_TIMEOUT_SECONDS,
fail_on_lock: false,
profile: Some(InvocationProfile::Ci),
}
.options(),
InvocationOptions {
timeout: None,
lock_timeout: None,
fail_on_lock: false,
jobs: None,
}
);
}

#[test]
fn disabled_deadline_allows_timeout_check() {
let _serial = deadline_test_lock()
Expand Down
12 changes: 12 additions & 0 deletions crates/no-mistakes/src/invocation/tests/napi_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ fn napi_missing_controls_disable_timeouts() {
assert_eq!(options.jobs, None);
}

#[test]
fn napi_profile_ci_is_stripped() {
let (json, options) = extract_napi_options(r#"{"profile":"ci","root":"."}"#).unwrap();
assert_eq!(options.timeout, None);
assert_eq!(options.lock_timeout, None);
assert_eq!(
serde_json::from_str::<Value>(&json).unwrap(),
serde_json::json!({"root":"."})
);
}

#[test]
fn napi_jobs_parses_non_negative_integer_or_null() {
let (_, options) = extract_napi_options(r#"{"jobs":4}"#).unwrap();
Expand All @@ -59,6 +70,7 @@ fn napi_controls_validate_types() {
r#"{"failOnLock":1}"#,
r#"{"jobs":-1}"#,
r#"{"jobs":"4"}"#,
r#"{"profile":"local"}"#,
"[]",
"not-json",
] {
Expand Down
1 change: 1 addition & 0 deletions crates/no-mistakes/src/napi_api/cli_parity_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub(crate) fn build_plan_args(options: TestsPlanOptions) -> AnyhowResult<crate::
format: Some(crate::tests::PlanFormat::Json),
json: true,
include_comment: options.include_comment,
include_glob: options.include_glob,
})
}

Expand Down
1 change: 1 addition & 0 deletions crates/no-mistakes/src/napi_api/options_flow_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub(crate) struct TestsPlanOptions {
pub(crate) global_config_fallback: Option<bool>,
pub(crate) direct_test_owner: bool,
pub(crate) include_comment: bool,
pub(crate) include_glob: Vec<String>,
}

#[derive(Debug, Default, Deserialize)]
Expand Down
4 changes: 4 additions & 0 deletions crates/no-mistakes/src/tests/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ pub(crate) struct PlanArgs {
/// Include the markdown PR comment on the plan JSON (`comment` field).
#[arg(long = "include-comment", default_value_t = false)]
pub(crate) include_comment: bool,

/// Keep only selected tests whose relative path matches one of these globs.
#[arg(long = "include-glob")]
pub(crate) include_glob: Vec<String>,
}

#[derive(Args, Debug, Clone)]
Expand Down
3 changes: 3 additions & 0 deletions crates/no-mistakes/src/tests/configured_plan/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ fn vitest_setup_args(root: PathBuf, changed_file: Vec<PathBuf>) -> PlanArgs {
format: None,
json: false,
include_comment: false,
include_glob: Vec::new(),
}
}

Expand Down Expand Up @@ -388,6 +389,7 @@ fn dependency_trigger_ignores_changed_test_discovery_errors_for_source_changes()
format: None,
json: false,
include_comment: false,
include_glob: Vec::new(),
};
let prepared = crate::tests::prepared_plan::PreparedTestPlanRequest::prepare(&plan_args)
.expect("fixture request should prepare");
Expand Down Expand Up @@ -481,6 +483,7 @@ fn explicit_ignored_changed_sources_impact_visible_tests_without_ignored_shadows
format: None,
json: false,
include_comment: false,
include_glob: Vec::new(),
})
.unwrap();
let selected = plan
Expand Down
3 changes: 3 additions & 0 deletions crates/no-mistakes/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ pub struct GroupedExecutionTarget {
pub config: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub project: Option<String>,
/// Path-prefix display name, such as a Swift package root.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub name: Option<String>,
pub base_command: Vec<String>,
/// Runner flags without test file paths.
pub runner_args: Vec<String>,
Expand Down
17 changes: 16 additions & 1 deletion crates/no-mistakes/src/tests/plan/changed_inventory.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{generate_plan_with_prepared_inner, PlanArgs, Result, TestPlan};
use crate::codebase::rules::path_filter::GlobMatcher;

pub(crate) fn generate_plan_with_prepared(
args: &PlanArgs,
Expand All @@ -7,6 +8,20 @@ pub(crate) fn generate_plan_with_prepared(
) -> Result<TestPlan> {
let mut plan = generate_plan_with_prepared_inner(args, prepared, timing)?;
plan.changed_files = prepared.changed_file_inventory();
plan.finish(args.include_comment);
retain_include_glob(&mut plan, &args.include_glob)?;
plan.finish(args.include_comment, &prepared.config.tests.swift.packages);
Ok(plan)
}

fn retain_include_glob(plan: &mut TestPlan, patterns: &[String]) -> Result<()> {
if patterns.is_empty() {
return Ok(());
}
let matcher = GlobMatcher::new(patterns, "includeGlob")?;
plan.selected_tests
.retain(|test| matcher.is_match(&test.test_file));
for group in &mut plan.groups {
group.selected.retain(|file| matcher.is_match(file));
}
Ok(())
}
30 changes: 26 additions & 4 deletions crates/no-mistakes/src/tests/plan_finish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,42 @@ use no_mistakes::codebase::test_discovery::TestExecutionTarget;
use std::collections::BTreeMap;

impl TestPlan {
pub(crate) fn finish(&mut self, include_comment: bool) {
self.execution_targets = grouped_execution_targets(&self.selected_tests);
pub(crate) fn finish(&mut self, include_comment: bool, prefixes: &[String]) {
self.execution_targets = grouped_execution_targets(&self.selected_tests, prefixes);
if include_comment {
self.comment = Some(super::comment::render_markdown_plan(self));
}
}
}

type ExecutionGroupKey = (String, Option<String>, Option<String>, Vec<String>);
type ExecutionGroupKey = (
String,
Option<String>,
Option<String>,
Vec<String>,
Option<String>,
);

fn grouped_execution_targets(selected: &[super::SelectedTest]) -> Vec<GroupedExecutionTarget> {
fn grouped_execution_targets(
selected: &[super::SelectedTest],
prefixes: &[String],
) -> Vec<GroupedExecutionTarget> {
let mut groups: BTreeMap<ExecutionGroupKey, GroupedExecutionTarget> = BTreeMap::new();
for test in selected {
for target in &test.targets {
let name = prefix_name(&test.test_file, prefixes);
let key = (
target.runner.clone(),
target.config.clone(),
target.project.clone(),
target.base_command.clone(),
name.clone(),
);
let group = groups.entry(key).or_insert_with(|| GroupedExecutionTarget {
runner: target.runner.clone(),
config: target.config.clone(),
project: target.project.clone(),
name,
base_command: target.base_command.clone(),
runner_args: runner_args_without_file(target, &test.test_file),
test_files: Vec::new(),
Expand All @@ -39,6 +51,16 @@ fn grouped_execution_targets(selected: &[super::SelectedTest]) -> Vec<GroupedExe
groups.into_values().collect()
}

fn prefix_name(file: &str, prefixes: &[String]) -> Option<String> {
prefixes
.iter()
.map(|prefix| prefix.trim_end_matches('/'))
.filter(|prefix| !prefix.is_empty())
.filter(|prefix| file == *prefix || file.starts_with(&format!("{prefix}/")))
.max_by_key(|prefix| prefix.len())
.map(str::to_string)
}

fn runner_args_without_file(target: &TestExecutionTarget, test_file: &str) -> Vec<String> {
let mut args = target.runner_args.clone();
if args.len() >= 2 && args[args.len() - 2] == "--test" {
Expand Down
56 changes: 44 additions & 12 deletions crates/no-mistakes/src/tests/plan_finish/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ fn selected(file: &str, targets: Vec<TestExecutionTarget>) -> SelectedTest {
/// `base_command`.
#[test]
fn mixed_python_runners_keep_separate_execution_targets() {
let groups = grouped_execution_targets(&[
selected(
"pkg/tests.py",
vec![target("python", Some("pkg"), &["python", "-m", "unittest"])],
),
selected(
"pkg/test_foo.py",
vec![target("python", Some("pkg"), &["pytest"])],
),
]);
let groups = grouped_execution_targets(
&[
selected(
"pkg/tests.py",
vec![target("python", Some("pkg"), &["python", "-m", "unittest"])],
),
selected(
"pkg/test_foo.py",
vec![target("python", Some("pkg"), &["pytest"])],
),
],
&[],
);

assert_eq!(groups.len(), 2);
let mut commands: Vec<_> = groups.into_iter().map(|group| group.base_command).collect();
Expand All @@ -63,12 +66,41 @@ fn nested_package_relative_runner_args_are_stripped_when_grouping() {
);
dart.runner_args = vec!["test/user_test.dart".into()];
dart.config = Some("packages/app".into());
let groups =
grouped_execution_targets(&[selected("packages/app/test/user_test.dart", vec![dart])]);
let groups = grouped_execution_targets(
&[selected("packages/app/test/user_test.dart", vec![dart])],
&[],
);
assert_eq!(groups.len(), 1);
assert!(groups[0].runner_args.is_empty());
assert_eq!(
groups[0].test_files,
vec!["packages/app/test/user_test.dart".to_string()]
);
}

#[test]
fn path_prefixes_split_and_name_execution_targets() {
let groups = grouped_execution_targets(
&[
selected(
"swift-clients/core/Tests/A.swift",
vec![target("swift", None, &["swift", "test"])],
),
selected(
"swift-clients/ui/Tests/B.swift",
vec![target("swift", None, &["swift", "test"])],
),
],
&["swift-clients/core".into(), "swift-clients/ui".into()],
);
assert_eq!(groups.len(), 2);
let mut names: Vec<_> = groups.into_iter().map(|group| group.name).collect();
names.sort();
assert_eq!(
names,
vec![
Some("swift-clients/core".into()),
Some("swift-clients/ui".into())
]
);
}
1 change: 1 addition & 0 deletions crates/no-mistakes/src/tests/plan_resources_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ fn resource_plan_args(root: &Path, changed: PathBuf) -> PlanArgs {
format: None,
json: true,
include_comment: false,
include_glob: Vec::new(),
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/no-mistakes/src/tests/prepared_plan/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ fn framework_args(root: &Path, framework: TestFramework) -> PlanArgs {
format: None,
json: false,
include_comment: false,
include_glob: Vec::new(),
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/no-mistakes/src/tests/why.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ fn run_live_analysis(
format: None,
json: true,
include_comment: false,
include_glob: Vec::new(),
};

let plan = generate_plan(&plan_args)?;
Expand Down
1 change: 1 addition & 0 deletions crates/no-mistakes/tests/cli_invocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ fn invocation_help_documents_independent_timeouts_and_lock_failure() {
assert!(help.contains("--timeout <SECONDS>"));
assert!(help.contains("--lock-timeout <SECONDS>"));
assert!(help.contains("--fail-on-lock"));
assert!(help.contains("--profile"));
assert!(help.contains("[default: 30]"));
}

Expand Down
Loading
Loading