From 268d22208abfb0a21d61de8a81a8350a52ece31f Mon Sep 17 00:00:00 2001 From: Cedric Barreteau Date: Fri, 19 Jul 2024 09:07:14 -0400 Subject: [PATCH] Add a `--build` flag to `buck2` test --- app/buck2_cli_proto/daemon.proto | 8 +++ app/buck2_client/src/commands/test.rs | 32 ++++++++++ app/buck2_test/src/command.rs | 88 +++++++++++++++++++-------- 3 files changed, 103 insertions(+), 25 deletions(-) diff --git a/app/buck2_cli_proto/daemon.proto b/app/buck2_cli_proto/daemon.proto index 090f9b31e135..35d0cd3b4770 100644 --- a/app/buck2_cli_proto/daemon.proto +++ b/app/buck2_cli_proto/daemon.proto @@ -472,6 +472,14 @@ message TestRequest { // Should you add tests that are on the `tests` attribute of the target. bool ignore_tests_attribute = 13; + + // Whether `DefaultInfo` providers for targets matching `target_patterns` + // should be built instead of only test ones. + bool build_default_info = 15; + + // Whether `RunInfo` providers for targets matching `target_patterns` + // should be built instead of only test ones. + bool build_run_info = 16; } message BxlRequest { diff --git a/app/buck2_client/src/commands/test.rs b/app/buck2_client/src/commands/test.rs index 2d0adacc803e..da825b99d18c 100644 --- a/app/buck2_client/src/commands/test.rs +++ b/app/buck2_client/src/commands/test.rs @@ -157,6 +157,36 @@ If include patterns are present, regardless of whether exclude patterns are pres #[clap(long)] test_executor_stderr: Option, + #[clap( + long, + group = "default-info", + help = "Also build default info (this is not the default)" + )] + build_default_info: bool, + + #[allow(unused)] + #[clap( + long, + group = "default-info", + help = "Do not build default info (this is the default)" + )] + skip_default_info: bool, + + #[clap( + long, + group = "run-info", + help = "Also build runtime dependencies (this is not the default)" + )] + build_run_info: bool, + + #[allow(unused)] + #[clap( + long, + group = "run-info", + help = "Do not build runtime dependencies (this is the default)" + )] + skip_run_info: bool, + /// Additional arguments passed to the test executor. /// /// Test executor is expected to have `--env` flag to pass environment variables. @@ -225,6 +255,8 @@ impl StreamingCommand for TestCommand { .transpose() .context("Invalid `timeout`")?, ignore_tests_attribute: self.ignore_tests_attribute, + build_default_info: self.build_default_info, + build_run_info: self.build_run_info, }, ctx.stdin() .console_interaction_stream(&self.common_opts.console_opts), diff --git a/app/buck2_test/src/command.rs b/app/buck2_test/src/command.rs index 4aee0e7fcdda..18ab26a29dde 100644 --- a/app/buck2_test/src/command.rs +++ b/app/buck2_test/src/command.rs @@ -19,7 +19,10 @@ use async_trait::async_trait; use buck2_build_api::analysis::calculation::RuleAnalysisCalculation; use buck2_build_api::artifact_groups::calculation::ArtifactGroupCalculation; use buck2_build_api::artifact_groups::ArtifactGroup; +use buck2_build_api::interpreter::rule_defs::cmd_args::CommandLineArgLike; use buck2_build_api::interpreter::rule_defs::cmd_args::SimpleCommandLineArtifactVisitor; +use buck2_build_api::interpreter::rule_defs::provider::builtin::default_info::FrozenDefaultInfo; +use buck2_build_api::interpreter::rule_defs::provider::builtin::run_info::FrozenRunInfo; use buck2_build_api::interpreter::rule_defs::provider::collection::FrozenProviderCollection; use buck2_build_api::interpreter::rule_defs::provider::test_provider::TestProvider; use buck2_cli_proto::HasClientContext; @@ -374,6 +377,8 @@ async fn test( MissingTargetBehavior::from_skip(build_opts.skip_missing_targets), timeout, request.ignore_tests_attribute, + request.build_default_info, + request.build_run_info, ) .await?; @@ -449,6 +454,8 @@ async fn test_targets( missing_target_behavior: MissingTargetBehavior, timeout: Option, ignore_tests_attribute: bool, + build_default_info: bool, + build_run_info: bool, ) -> anyhow::Result { let session = Arc::new(session); @@ -532,6 +539,8 @@ async fn test_targets( working_dir_cell, missing_target_behavior, ignore_tests_attribute, + build_default_info, + build_run_info, }); driver.push_pattern( @@ -658,6 +667,8 @@ pub(crate) struct TestDriverState<'a, 'e> { working_dir_cell: CellName, missing_target_behavior: MissingTargetBehavior, ignore_tests_attribute: bool, + build_default_info: bool, + build_run_info: bool, } /// Maintains the state of an ongoing test execution. @@ -840,6 +851,8 @@ impl<'a, 'e> TestDriver<'a, 'e> { state.label_filtering.dupe(), state.cell_resolver, state.working_dir_cell, + state.build_default_info, + state.build_run_info, ) .await?; anyhow::Ok(vec![]) @@ -894,17 +907,26 @@ async fn test_target( label_filtering: Arc, cell_resolver: &CellResolver, working_dir_cell: CellName, + build_default_info: bool, + build_run_info: bool, ) -> anyhow::Result> { // NOTE: We fail if we hit an incompatible target here. This can happen if we reach an // incompatible target via `tests = [...]`. This should perhaps change, but that's how it works // in v1: https://fb.workplace.com/groups/buckeng/posts/8520953297953210 let frozen_providers = ctx.get_providers(&target).await?.require_compatible()?; let providers = frozen_providers.provider_collection(); - build_artifacts(ctx, providers, &label_filtering).await?; + build_artifacts( + ctx, + providers, + &label_filtering, + build_default_info, + build_run_info, + ) + .await?; let fut = match ::from_collection(providers) { Some(test_info) => { - if skip_run_based_on_labels(test_info, &label_filtering) { + if label_filtering.is_excluded(test_info.labels()) { return Ok(None); } run_tests( @@ -930,46 +952,62 @@ async fn test_target( fut.await } -fn skip_run_based_on_labels( - provider: &dyn TestProvider, - label_filtering: &TestLabelFiltering, -) -> bool { - let target_labels = provider.labels(); - label_filtering.is_excluded(target_labels) -} - -fn skip_build_based_on_labels( - provider: &dyn TestProvider, +fn skip_build_based_on_labels<'a>( + labels_fn: &dyn Fn() -> Vec<&'a str>, label_filtering: &TestLabelFiltering, ) -> bool { - !label_filtering.build_filtered_targets && skip_run_based_on_labels(provider, label_filtering) + !label_filtering.build_filtered_targets && label_filtering.is_excluded(labels_fn()) } async fn build_artifacts( ctx: &mut DiceComputations<'_>, providers: &FrozenProviderCollection, label_filtering: &TestLabelFiltering, + build_default_info: bool, + build_run_info: bool, ) -> anyhow::Result<()> { fn get_artifacts_to_build( label_filtering: &TestLabelFiltering, providers: &FrozenProviderCollection, + build_default_info: bool, + build_run_info: bool, ) -> anyhow::Result> { - Ok(match ::from_collection(providers) { - Some(provider) => { - if skip_build_based_on_labels(provider, label_filtering) { - return Ok(indexset![]); - } + let mut artifacts = IndexSet::new(); + + if let Some(test_provider) = ::from_collection(providers) { + if skip_build_based_on_labels(&|| test_provider.labels(), label_filtering) { + return Ok(indexset![]); + } + let mut artifact_visitor = SimpleCommandLineArtifactVisitor::new(); + test_provider.visit_artifacts(&mut artifact_visitor)?; + artifacts.extend(artifact_visitor.inputs) + } + + if build_default_info { + if let Some(provider) = providers.builtin_provider::() { + provider.for_each_output(&mut |artifact| { + artifacts.insert(artifact); + })?; + } + } + + if build_run_info { + if let Some(provider) = providers.builtin_provider::() { let mut artifact_visitor = SimpleCommandLineArtifactVisitor::new(); provider.visit_artifacts(&mut artifact_visitor)?; - artifact_visitor.inputs + artifacts.extend(artifact_visitor.inputs); } - None => { - // not a test - indexset![] - } - }) + } + + Ok(artifacts) } - let artifacts_to_build = get_artifacts_to_build(label_filtering, providers)?; + + let artifacts_to_build = get_artifacts_to_build( + label_filtering, + providers, + build_default_info, + build_run_info, + )?; // build the test target first ctx.try_compute_join(artifacts_to_build.iter(), |ctx, input| { ctx.ensure_artifact_group(input).boxed()