diff --git a/README.md b/README.md index e74c87e..e577712 100644 --- a/README.md +++ b/README.md @@ -399,7 +399,10 @@ the tests that reach them, across project boundaries. runs `… exec mix test --partitions 2 test/a_test.exs test/b_test.exs` with the tests selected in `//core`, so each shard takes its slice of the - selection; `vitest --shard=1/2` works the same way. Everything before the + selection; `vitest --shard=1/2` works the same way. A `mix test` shard + whose partition gets none of the selected files is skipped: partition 3 + of two files is empty, and Mix fails on paths that match nothing. + Everything before the final command is kept, which must be a plain command after an optional `cd &&`. Test targets the requested target depends on in other projects are narrowed too, each to its own project's selection, so a diff --git a/src/cli/affected_files.rs b/src/cli/affected_files.rs index 70ebb53..40a16fa 100644 --- a/src/cli/affected_files.rs +++ b/src/cli/affected_files.rs @@ -442,8 +442,16 @@ impl<'a> RelatedPlanner<'a> { && def.command.contains("{files}") && def.capabilities.contains(&TargetCapability::FilesList); let plugin = self.registry.find_by_name(&project.plugin_name); + // The plugin sees the environment a one-liner sets for the command, + // which the one-liner itself keeps. + let environment = runner + .delegated + .as_ref() + .map(|delegated| delegated.environment()) + .unwrap_or_default(); + let command = format!("{environment}{}", runner.command); let attempt = |tests: &[RelatedTest]| { - plugin.and_then(|p| p.related_tests(&project.root, &runner.command, tests)) + plugin.and_then(|p| p.related_tests(&project.root, &command, tests)) }; if !placeholder && attempt(&[]).is_none() { return Ok(Narrowed::NotRunner); @@ -480,7 +488,9 @@ impl<'a> RelatedPlanner<'a> { let commands: Vec = commands .iter() .map(|command| match &runner.delegated { - Some(delegated) => delegated.render(command), + Some(delegated) => { + delegated.render(command.strip_prefix(&environment).unwrap_or(command)) + } None => command.clone(), }) .collect(); diff --git a/src/plugins/related_tests.rs b/src/plugins/related_tests.rs index a36e7fc..aa3c966 100644 --- a/src/plugins/related_tests.rs +++ b/src/plugins/related_tests.rs @@ -219,6 +219,22 @@ impl Delegated { }) } + /// The environment assignments written before the test command + /// (`A=1 exec mix test` has `A=1`), each followed by a space. + pub(crate) fn environment(&self) -> String { + let segment = self + .head + .rsplit("&&") + .next() + .and_then(|rest| rest.rsplit(';').next()) + .unwrap_or(""); + segment + .split_whitespace() + .filter(|word| is_assignment(word)) + .map(|word| format!("{word} ")) + .collect() + } + /// The one-liner with its test command replaced. pub(crate) fn render(&self, command: &str) -> String { let script = format!("{}{command}", self.head); @@ -321,9 +337,38 @@ pub(super) fn mix( if keep.is_empty() { return plan(Vec::new()); } + // `--partitions N` deals the files it is given out in turn, and + // partition K of fewer than K files gets none: Mix then fails, saying + // the paths match nothing. + if let Some(partition) = mix_partition(&argv, test) { + let given: BTreeSet<&String> = keep.iter().collect(); + if given.len() < partition { + return plan(Vec::new()); + } + } plan(vec![argv.render(&listed, &keep)]) } +/// Which partition a `mix test --partitions N` command runs, when +/// `MIX_TEST_PARTITION` is written in front of it. +fn mix_partition(argv: &Argv, test: usize) -> Option { + let arguments = &argv.parts[test + 1..]; + let total: usize = arguments.iter().enumerate().find_map(|(i, part)| { + match part.strip_prefix("--partitions") { + Some("") => arguments.get(i + 1)?.parse().ok(), + Some(rest) => rest.strip_prefix('=')?.parse().ok(), + None => None, + } + })?; + let partition: usize = argv.parts[..argv.program] + .iter() + .rev() + .find_map(|part| part.strip_prefix("MIX_TEST_PARTITION="))? + .parse() + .ok()?; + (total > 1 && (1..=total).contains(&partition)).then_some(partition) +} + const JS_RUNNERS: &[&str] = &["vitest", "jest", "mocha"]; const JS_LAUNCHERS: &[&str] = &[ "npx", diff --git a/tests/related.rs b/tests/related.rs index d96e11a..48adae7 100644 --- a/tests/related.rs +++ b/tests/related.rs @@ -1161,6 +1161,9 @@ fn shard_projects_run_their_slice_of_the_selected_tests() { "Enum.sum(items)", "Enum.sum(items) + 0", ); + // Both pricing functions changed, so two test files are selected. Mix + // deals them out, one to each partition. + ws.edit("shop/lib/shop/pricing.ex", "amount * 0.2", "amount * 0.25"); let plan = ws.plan("test-ci"); for shard in ["1", "2"] { let commands = plan.commands(&format!("//shop-shard-{shard}:test-ci")); @@ -1172,7 +1175,7 @@ fn shard_projects_run_their_slice_of_the_selected_tests() { "-c".to_string(), format!( "cd ../shop && MIX_TEST_PARTITION={shard} exec mix test --warnings-as-errors \ - --partitions 2 test/shop/cart_test.exs" + --partitions 2 test/shop/cart_test.exs test/shop/tax_test.exs" ), ] ); @@ -1184,6 +1187,27 @@ fn shard_projects_run_their_slice_of_the_selected_tests() { ); } +#[test] +fn a_shard_whose_partition_gets_no_selected_file_is_skipped() { + let ws = Workspace::new(&sharded()); + ws.edit( + "shop/lib/shop/pricing.ex", + "Enum.sum(items)", + "Enum.sum(items) + 0", + ); + // One file is selected. `mix test --partitions 2` gives it to the first + // partition; the second would fail, saying its paths match nothing. + let plan = ws.plan("test-ci"); + assert_eq!( + plan.commands("//shop-shard-1:test-ci"), + ["bash -c 'cd ../shop && MIX_TEST_PARTITION=1 exec mix test --warnings-as-errors --partitions 2 test/shop/cart_test.exs'"] + ); + assert!( + !plan.runs().iter().any(|addr| addr.contains("shop-shard-2")), + "{plan:#?}" + ); +} + #[test] fn shard_projects_are_skipped_when_no_test_reaches_the_change() { let ws = Workspace::new(&sharded()); @@ -1236,8 +1260,12 @@ fn shard_projects_run_in_full_when_their_own_files_change() { assert!(plan .notes("//shop-shard-1:test-ci") .contains("shop-shard-1/aster.toml changed; running in full")); - // The other shard still narrows. - assert_eq!(plan.commands("//shop-shard-2:test-ci").len(), 1); + // The other shard still narrows: to nothing, as the one selected file + // is not in its partition. + assert!( + !plan.runs().iter().any(|addr| addr.contains("shop-shard-2")), + "{plan:#?}" + ); } #[test]