diff --git a/Cargo.lock b/Cargo.lock index 3f45b87ecc5..e738563c3a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1756,10 +1756,12 @@ dependencies = [ "serde_json", "sha2", "shell-words", + "tar", "target-spec", "target-spec-miette", "tokio", "whoami", + "zstd", ] [[package]] diff --git a/cargo-nextest/src/dispatch.rs b/cargo-nextest/src/dispatch.rs index 3588aa4f1ea..326055690b2 100644 --- a/cargo-nextest/src/dispatch.rs +++ b/cargo-nextest/src/dispatch.rs @@ -45,7 +45,9 @@ use nextest_runner::{ show_config::{ShowNextestVersion, ShowTestGroupSettings, ShowTestGroups, ShowTestGroupsMode}, signal::SignalHandlerKind, target_runner::{PlatformRunner, TargetRunner}, - test_filter::{FilterBound, RunIgnored, TestFilterBuilder, TestFilterPatterns}, + test_filter::{ + FilterBinaryMatch, FilterBound, RunIgnored, TestFilterBuilder, TestFilterPatterns, + }, test_output::CaptureStrategy, write_str::WriteStr, }; @@ -188,6 +190,7 @@ impl AppOpts { cargo_options, archive_file, archive_format, + build_filter, zstd_level, } => { let app = BaseApp::new( @@ -198,6 +201,7 @@ impl AppOpts { self.common.manifest_path, output_writer, )?; + let app = App::new(app, build_filter)?; app.exec_archive(&archive_file, archive_format, zstd_level, output_writer)?; Ok(0) } @@ -390,6 +394,9 @@ enum Command { )] archive_format: ArchiveFormatOpt, + #[clap(flatten)] + build_filter: TestBuildFilter, + /// Zstandard compression level (-7 to 22, higher is more compressed + slower) #[arg( long, @@ -1519,69 +1526,6 @@ impl BaseApp { }) } - fn exec_archive( - &self, - output_file: &Utf8Path, - format: ArchiveFormatOpt, - zstd_level: i32, - output_writer: &mut OutputWriter, - ) -> Result<()> { - // Do format detection first so we fail immediately. - let format = format.to_archive_format(output_file)?; - let binary_list = self.build_binary_list()?; - let path_mapper = PathMapper::noop(); - - let build_platforms = binary_list.rust_build_meta.build_platforms.clone(); - let pcx = ParseContext::new(self.graph()); - let (_, config) = self.load_config(&pcx)?; - let profile = self - .load_profile(&config)? - .apply_build_platforms(&build_platforms); - - let redactor = if should_redact() { - Redactor::build_active(&binary_list.rust_build_meta) - .with_path(output_file.to_path_buf(), "".to_owned()) - .build() - } else { - Redactor::noop() - }; - - let mut reporter = ArchiveReporter::new(self.output.verbose, redactor.clone()); - if self - .output - .color - .should_colorize(supports_color::Stream::Stderr) - { - reporter.colorize(); - } - - let mut writer = output_writer.stderr_writer(); - archive_to_file( - profile, - &binary_list, - &self.cargo_metadata_json, - &self.package_graph, - // Note that path_mapper is currently a no-op -- we don't support reusing builds for - // archive creation because it's too confusing. - &path_mapper, - format, - zstd_level, - output_file, - |event| { - reporter.report_event(event, &mut writer)?; - writer.flush() - }, - redactor.clone(), - ) - .map_err(|err| ExpectedError::ArchiveCreateError { - archive_file: output_file.to_owned(), - err, - redactor, - })?; - - Ok(()) - } - fn build_binary_list(&self) -> Result> { let binary_list = match self.reuse_build.binaries_metadata() { Some(m) => m.binary_list.clone(), @@ -1661,12 +1605,16 @@ impl App { Ok(Self { base, build_filter }) } - fn build_filtering_expressions(&self, pcx: &ParseContext<'_>) -> Result> { + fn build_filtering_expressions( + &self, + pcx: &ParseContext<'_>, + kind: FiltersetKind, + ) -> Result> { let (exprs, all_errors): (Vec<_>, Vec<_>) = self .build_filter .filterset .iter() - .map(|input| Filterset::parse(input.clone(), pcx, FiltersetKind::Test)) + .map(|input| Filterset::parse(input.clone(), pcx, kind)) .partition_result(); if !all_errors.is_empty() { @@ -1706,7 +1654,7 @@ impl App { let (version_only_config, config) = self.base.load_config(&pcx)?; let profile = self.base.load_profile(&config)?; - let filter_exprs = self.build_filtering_expressions(&pcx)?; + let filter_exprs = self.build_filtering_expressions(&pcx, FiltersetKind::Test)?; let test_filter_builder = self.build_filter.make_test_filter_builder(filter_exprs)?; let binary_list = self.base.build_binary_list()?; @@ -1758,6 +1706,123 @@ impl App { Ok(()) } + fn exec_archive( + &self, + output_file: &Utf8Path, + format: ArchiveFormatOpt, + zstd_level: i32, + output_writer: &mut OutputWriter, + ) -> Result<()> { + // Do format detection first so we fail immediately. + let format = format.to_archive_format(output_file)?; + let binary_list = self.base.build_binary_list()?; + let path_mapper = PathMapper::noop(); + let build_platforms = &binary_list.rust_build_meta.build_platforms; + let pcx = ParseContext::new(self.base.graph()); + let (_, config) = self.base.load_config(&pcx)?; + let profile = self + .base + .load_profile(&config)? + .apply_build_platforms(build_platforms); + let redactor = if should_redact() { + Redactor::build_active(&binary_list.rust_build_meta) + .with_path(output_file.to_path_buf(), "".to_owned()) + .build() + } else { + Redactor::noop() + }; + let mut reporter = ArchiveReporter::new(self.base.output.verbose, redactor.clone()); + + if self + .base + .output + .color + .should_colorize(supports_color::Stream::Stderr) + { + reporter.colorize(); + } + + let rust_build_meta = binary_list.rust_build_meta.map_paths(&path_mapper); + let test_artifacts = RustTestArtifact::from_binary_list( + self.base.graph(), + binary_list.clone(), + &rust_build_meta, + &path_mapper, + self.build_filter.platform_filter.into(), + )?; + + let filter_bound = { + if self.build_filter.ignore_default_filter { + FilterBound::All + } else { + FilterBound::DefaultSet + } + }; + + let filter_exprs = self.build_filtering_expressions(&pcx, FiltersetKind::TestArchive)?; + let test_filter_builder = self.build_filter.make_test_filter_builder(filter_exprs)?; + + let ecx = profile.filterset_ecx(); + + // Apply filterset to `RustTestArtifact` list. + let test_artifacts: BTreeSet<_> = test_artifacts + .iter() + .filter(|test_artifact| { + matches!( + test_filter_builder.filter_binary_match(test_artifact, &ecx, filter_bound), + FilterBinaryMatch::Definite | FilterBinaryMatch::Possible + ) + }) + .map(|test_artifact| &test_artifact.binary_id) + .collect(); + + let filtered_binaries: Vec<_> = binary_list + .rust_binaries + .iter() + .filter(|binary| test_artifacts.contains(&binary.id)) + .cloned() + .collect(); + + if filtered_binaries.len() < binary_list.rust_binaries.len() { + info!( + "archiving {} of {} test binaries after filtering", + filtered_binaries.len(), + binary_list.rust_binaries.len() + ); + } + + let binary_list_to_archive = BinaryList { + rust_build_meta: binary_list.rust_build_meta.clone(), + rust_binaries: filtered_binaries, + }; + + let mut writer = output_writer.stderr_writer(); + archive_to_file( + profile, + &binary_list_to_archive, + &self.base.cargo_metadata_json, + &self.base.package_graph, + // Note that path_mapper is currently a no-op -- we don't support reusing builds for + // archive creation because it's too confusing. + &path_mapper, + format, + zstd_level, + output_file, + |event| { + reporter.report_event(event, &mut writer)?; + writer.flush() + }, + redactor.clone(), + ) + .map_err(|err| ExpectedError::ArchiveCreateError { + archive_file: output_file.to_owned(), + err, + redactor, + })?; + + Ok(()) + } + fn exec_show_test_groups( &self, show_default: bool, @@ -1777,7 +1842,7 @@ impl App { }; let settings = ShowTestGroupSettings { mode, show_default }; - let filter_exprs = self.build_filtering_expressions(&pcx)?; + let filter_exprs = self.build_filtering_expressions(&pcx, FiltersetKind::Test)?; let test_filter_builder = self.build_filter.make_test_filter_builder(filter_exprs)?; let binary_list = self.base.build_binary_list()?; @@ -1874,7 +1939,7 @@ impl App { reporter_opts.to_builder(runner_opts.no_run, no_capture, should_colorize); reporter_builder.set_verbose(self.base.output.verbose); - let filter_exprs = self.build_filtering_expressions(&pcx)?; + let filter_exprs = self.build_filtering_expressions(&pcx, FiltersetKind::Test)?; let test_filter_builder = self.build_filter.make_test_filter_builder(filter_exprs)?; let binary_list = self.base.build_binary_list()?; diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 0e84146573a..82c6a3151a0 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -49,6 +49,8 @@ serde_json.workspace = true sha2.workspace = true shell-words.workspace = true whoami.workspace = true +zstd.workspace = true +tar.workspace = true [dev-dependencies] camino-tempfile-ext.workspace = true diff --git a/integration-tests/tests/integration/main.rs b/integration-tests/tests/integration/main.rs index 9c3636e487b..e0c19165542 100644 --- a/integration-tests/tests/integration/main.rs +++ b/integration-tests/tests/integration/main.rs @@ -23,12 +23,13 @@ //! `NEXTEST_BIN_EXE_cargo-nextest-dup`. use camino::{Utf8Path, Utf8PathBuf}; +use fixture_data::nextest_tests::EXPECTED_TEST_SUITES; use integration_tests::{ env::set_env_vars, nextest_cli::{CargoNextestCli, CargoNextestOutput}, }; use nextest_metadata::{BuildPlatform, NextestExitCode, TestListSummary}; -use std::{borrow::Cow, fs::File, io::Write}; +use std::{borrow::Cow, ffi::OsStr, fs::File, io::Write, path::PathBuf}; use target_spec::{Platform, summaries::TargetFeaturesSummary}; mod fixtures; @@ -842,6 +843,113 @@ archive.include = [ .expect_err("archive should have failed"); } +#[test] +fn test_archive_with_build_filter() { + set_env_vars(); + + let all_test_binaries: Vec = EXPECTED_TEST_SUITES + .iter() + // The test fixture binary name uses hyphens, but the files will use an underscore. + .map(|fixture| fixture.binary_name.replace("-", "_")) + .collect(); + + // Check that all test files are present with the `all()` filter. + check_archive_contents("all()", |paths| { + for file in all_test_binaries.iter() { + assert!( + paths + .iter() + .any(|path| path_contains_test_fixture_file(path, file)), + "{:?} was missing from the test archive", + file + ); + } + }); + + // Check that no test files are present with the `none()` filter. + check_archive_contents("none()", |paths| { + for file in all_test_binaries.iter() { + assert!( + !paths + .iter() + .filter(|path| path + .ancestors() + // Test files are in the `deps` folder. + .any(|folder| folder.file_name() == Some(OsStr::new("deps")))) + .any(|path| path_contains_test_fixture_file(path, file)), + "{:?} was present in the test archive but it should be missing", + file + ); + } + }); + + let expected_package_test_file = "cdylib_example"; + let filtered_test = "nextest_tests"; + // Check that test files are filtered by the `package()` filter. + check_archive_contents("package(cdylib-example)", |paths| { + assert!( + paths + .iter() + .any(|path| path_contains_test_fixture_file(path, expected_package_test_file)), + "{:?} was missing from the test archive", + expected_package_test_file + ); + assert!( + !paths + .iter() + .any(|path| path_contains_test_fixture_file(path, filtered_test)), + "{:?} was present in the test archive but it should be missing", + filtered_test + ); + }); +} + +/// Checks if the file name at `path` contains `expected_file_name` +/// Returns `true` if it does, otherwise `false`. +fn path_contains_test_fixture_file(path: &PathBuf, expected_file_name: &str) -> bool { + let file_name = path.file_name().unwrap_or_else(|| { + panic!( + "test fixture path {:?} did not have a file name, does the path contain '..'?", + path + ) + }); + file_name + .to_str() + .unwrap_or_else(|| panic!("file name: {:?} is not valid utf-8", file_name)) + .contains(expected_file_name) +} + +#[test] +fn test_archive_with_unsupported_test_filter() { + set_env_vars(); + + let unsupported_filter = "test(sample_test)"; + assert!( + create_archive_with_args( + "", + false, + "archive_unsupported_build_filter", + &["-E", unsupported_filter], + true + ) + .is_err() + ); +} + +fn check_archive_contents(filter: &str, cb: impl FnOnce(Vec)) { + let (_p1, archive_file) = + create_archive_with_args("", false, "", &["-E", filter], false).expect("archive succeeded"); + let file = File::open(archive_file).unwrap(); + let decoder = zstd::stream::read::Decoder::new(file).unwrap(); + let mut archive = tar::Archive::new(decoder); + let paths = archive + .entries() + .unwrap() + .map(|e| e.unwrap().path().unwrap().into_owned()) + .collect::>(); + cb(paths); +} + const APP_DATA_DIR: &str = "application-data"; // The default limit is 16, so anything at depth 17 (under d16) is excluded. const DIR_TREE: &str = "application-data/d1/d2/d3/d4/d5/d6/d7/d8/d9/d10/d11/d12/d13/d14/d15/d16"; @@ -860,6 +968,16 @@ fn create_archive( config_contents: &str, make_uds: bool, snapshot_name: &str, +) -> Result<(TempProject, Utf8PathBuf), CargoNextestOutput> { + create_archive_with_args(config_contents, make_uds, snapshot_name, &[], true) +} + +fn create_archive_with_args( + config_contents: &str, + make_uds: bool, + snapshot_name: &str, + extra_args: &[&str], + check_output: bool, ) -> Result<(TempProject, Utf8PathBuf), CargoNextestOutput> { let custom_target_dir = Utf8TempDir::new().unwrap(); let custom_target_path = custom_target_dir.path().join("target"); @@ -894,22 +1012,26 @@ fn create_archive( let archive_file = p.temp_root().join("my-archive.tar.zst"); + let manifest_path = p.manifest_path(); + let mut cli_args = vec![ + "--manifest-path", + manifest_path.as_str(), + "archive", + "--archive-file", + archive_file.as_str(), + "--workspace", + "--target-dir", + p.target_dir().as_str(), + "--all-targets", + // Make cargo fully quiet since we're testing just nextest output below. + "--cargo-quiet", + "--cargo-quiet", + ]; + cli_args.extend(extra_args); + // Write the archive to the archive_file above. let output = CargoNextestCli::for_test() - .args([ - "--manifest-path", - p.manifest_path().as_str(), - "archive", - "--archive-file", - archive_file.as_str(), - "--workspace", - "--target-dir", - p.target_dir().as_str(), - "--all-targets", - // Make cargo fully quiet since we're testing just nextest output below. - "--cargo-quiet", - "--cargo-quiet", - ]) + .args(cli_args) .env("__NEXTEST_REDACT", "1") // Used for linked path testing. See comment in // binary_list.rs:detect_linked_path. @@ -923,7 +1045,9 @@ fn create_archive( UdsStatus::NotCreated => format!("{snapshot_name}_without_uds"), }; - insta::assert_snapshot!(snapshot_name, output.stderr_as_str()); + if check_output { + insta::assert_snapshot!(snapshot_name, output.stderr_as_str()); + } // Remove the old source and target directories to ensure that any tests that refer to files within // it fail. diff --git a/integration-tests/tests/integration/snapshots/integration__archive_unsupported_build_filter.snap b/integration-tests/tests/integration/snapshots/integration__archive_unsupported_build_filter.snap new file mode 100644 index 00000000000..81829ff80b6 --- /dev/null +++ b/integration-tests/tests/integration/snapshots/integration__archive_unsupported_build_filter.snap @@ -0,0 +1,12 @@ +--- +source: integration-tests/tests/integration/main.rs +expression: output.stderr_as_str() +--- + error: predicate not allowed in `test-archive-filter` expressions + ╭──── + 1 │ test(sample_test) + · ─────┬───── + · ╰── this predicate is banned because test predicates are not supported while archiving + ╰──── + +error: failed to parse filterset diff --git a/nextest-filtering/src/compile.rs b/nextest-filtering/src/compile.rs index bb917ba9659..2392d5f3ab2 100644 --- a/nextest-filtering/src/compile.rs +++ b/nextest-filtering/src/compile.rs @@ -41,6 +41,19 @@ fn check_banned_predicates( ) { match kind { FiltersetKind::Test => {} + FiltersetKind::TestArchive => { + // The `test` predicate is unsupported for a test archive since we need to + // package the whole binary and it may be cross-compiled. + Wrapped(expr).collapse_frames(|layer: ExprFrame<&ParsedLeaf, ()>| { + if let ExprFrame::Set(ParsedLeaf::Test(_, span)) = layer { + errors.push(ParseSingleError::BannedPredicate { + kind, + span: *span, + reason: BannedPredicateReason::Unsupported, + }); + } + }) + } FiltersetKind::DefaultFilter => { // The `default` predicate is banned. Wrapped(expr).collapse_frames(|layer: ExprFrame<&ParsedLeaf, ()>| { diff --git a/nextest-filtering/src/errors.rs b/nextest-filtering/src/errors.rs index 9b18d94657a..915c196293c 100644 --- a/nextest-filtering/src/errors.rs +++ b/nextest-filtering/src/errors.rs @@ -59,7 +59,7 @@ pub enum ParseSingleError { kind: FiltersetKind, /// The span of the banned predicate. - #[label("this predicate causes {reason}")] + #[label("this predicate is banned because {reason}")] span: SourceSpan, /// The reason why the predicate is banned. @@ -130,6 +130,10 @@ pub enum ParseSingleError { #[error("invalid argument for platform")] InvalidPlatformArgument(#[label("expected \"target\" or \"host\"")] SourceSpan), + /// Contained an unsupported expression. + #[error("unsupported expression")] + UnsupportedExpression(#[label("contained an unsupported expression")] SourceSpan), + /// An unknown parsing error occurred. #[error("unknown parsing error")] Unknown, @@ -193,13 +197,18 @@ impl<'a> State<'a> { pub enum BannedPredicateReason { /// This predicate causes infinite recursion. InfiniteRecursion, + /// This predicate is unsupported. + Unsupported, } impl fmt::Display for BannedPredicateReason { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { BannedPredicateReason::InfiniteRecursion => { - write!(f, "infinite recursion") + write!(f, "it causes infinite recursion") + } + BannedPredicateReason::Unsupported => { + write!(f, "test predicates are not supported while archiving") } } } diff --git a/nextest-filtering/src/expression.rs b/nextest-filtering/src/expression.rs index 18c66a44551..71fefce2a5d 100644 --- a/nextest-filtering/src/expression.rs +++ b/nextest-filtering/src/expression.rs @@ -369,6 +369,9 @@ pub enum FiltersetKind { /// A test filterset. Test, + /// A test archive filterset. + TestArchive, + /// A default-filter filterset. /// /// To prevent recursion, default-filter expressions cannot contain `default()` themselves. @@ -380,6 +383,7 @@ impl fmt::Display for FiltersetKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Test => write!(f, "test"), + Self::TestArchive => write!(f, "test-archive-filter"), Self::DefaultFilter => write!(f, "default-filter"), } } diff --git a/nextest-runner/src/config/overrides/imp.rs b/nextest-runner/src/config/overrides/imp.rs index e91ede36337..3a454ffdf13 100644 --- a/nextest-runner/src/config/overrides/imp.rs +++ b/nextest-runner/src/config/overrides/imp.rs @@ -1247,7 +1247,7 @@ mod tests { message: "predicate not allowed in `default-filter` expressions".to_owned(), labels: vec![ MietteJsonLabel { - label: "this predicate causes infinite recursion".to_owned(), + label: "this predicate is banned because it causes infinite recursion".to_owned(), span: MietteJsonSpan { offset: 4, length: 9 }, }, ], @@ -1342,7 +1342,7 @@ mod tests { &[MietteJsonReport { message: "predicate not allowed in `default-filter` expressions".to_owned(), labels: vec![ - MietteJsonLabel { label: "this predicate causes infinite recursion".to_owned(), span: MietteJsonSpan { offset: 13, length: 9 } } + MietteJsonLabel { label: "this predicate is banned because it causes infinite recursion".to_owned(), span: MietteJsonSpan { offset: 13, length: 9 } } ] }]