From 42f6002d33442a0229bcce10a6da8a3dc8c57973 Mon Sep 17 00:00:00 2001 From: Cedric Barreteau Date: Thu, 20 Jun 2024 11:57:07 -0400 Subject: [PATCH] Add a `--build` flag to `buck2` test --- app/buck2_cli_proto/daemon.proto | 3 ++ app/buck2_client/src/commands/test.rs | 4 ++ app/buck2_test/src/command.rs | 60 +++++++++++++++------------ 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/app/buck2_cli_proto/daemon.proto b/app/buck2_cli_proto/daemon.proto index fe16274b1a99..75db86a9d39a 100644 --- a/app/buck2_cli_proto/daemon.proto +++ b/app/buck2_cli_proto/daemon.proto @@ -472,6 +472,9 @@ message TestRequest { // Should you add tests that are on the `tests` attribute of the target. bool ignore_tests_attribute = 13; + + // Whether all targets matching `target_patterns` should be built instead of only test ones. + bool build = 15; } message BxlRequest { diff --git a/app/buck2_client/src/commands/test.rs b/app/buck2_client/src/commands/test.rs index 4cc8c9695faf..8710a3c3fe08 100644 --- a/app/buck2_client/src/commands/test.rs +++ b/app/buck2_client/src/commands/test.rs @@ -157,6 +157,9 @@ If include patterns are present, regardless of whether exclude patterns are pres #[clap(long)] test_executor_stderr: Option, + #[clap(long)] + build: bool, + /// Additional arguments passed to the test executor. /// /// Test executor is expected to have `--env` flag to pass environment variables. @@ -225,6 +228,7 @@ impl StreamingCommand for TestCommand { .transpose() .context("Invalid `timeout`")?, ignore_tests_attribute: self.ignore_tests_attribute, + build: self.build, }, 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 505b585f8883..062989bb5314 100644 --- a/app/buck2_test/src/command.rs +++ b/app/buck2_test/src/command.rs @@ -20,6 +20,7 @@ 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::SimpleCommandLineArtifactVisitor; +use buck2_build_api::interpreter::rule_defs::provider::builtin::default_info::FrozenDefaultInfo; 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; @@ -354,6 +355,7 @@ async fn test( .transpose() .context("Invalid `duration`")?; + let only_build_tests = !request.build; let test_outcome = test_targets( ctx, resolved_pattern, @@ -373,6 +375,7 @@ async fn test( MissingTargetBehavior::from_skip(build_opts.skip_missing_targets), timeout, request.ignore_tests_attribute, + only_build_tests, ) .await?; @@ -448,6 +451,7 @@ async fn test_targets( missing_target_behavior: MissingTargetBehavior, timeout: Option, ignore_tests_attribute: bool, + only_build_tests: bool, ) -> anyhow::Result { let session = Arc::new(session); @@ -531,6 +535,7 @@ async fn test_targets( working_dir_cell, missing_target_behavior, ignore_tests_attribute, + only_build_tests, }); driver.push_pattern( @@ -657,6 +662,7 @@ pub(crate) struct TestDriverState<'a, 'e> { working_dir_cell: CellName, missing_target_behavior: MissingTargetBehavior, ignore_tests_attribute: bool, + only_build_tests: bool, } /// Maintains the state of an ongoing test execution. @@ -839,6 +845,7 @@ impl<'a, 'e> TestDriver<'a, 'e> { state.label_filtering.dupe(), state.cell_resolver, state.working_dir_cell, + state.only_build_tests, ) .await?; anyhow::Ok(vec![]) @@ -893,17 +900,18 @@ async fn test_target( label_filtering: Arc, cell_resolver: &CellResolver, working_dir_cell: CellName, + only_build_tests: 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, only_build_tests).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( @@ -929,46 +937,46 @@ async fn test_target( fut.await } -fn skip_run_based_on_labels( - provider: &dyn TestProvider, +fn skip_build_based_on_labels<'a>( + labels_fn: &dyn Fn() -> Vec<&'a str>, label_filtering: &TestLabelFiltering, ) -> bool { - let target_labels = provider.labels(); - label_filtering.is_excluded(target_labels) -} - -fn skip_build_based_on_labels( - provider: &dyn TestProvider, - 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, + only_build_tests: bool, ) -> anyhow::Result<()> { fn get_artifacts_to_build( label_filtering: &TestLabelFiltering, providers: &FrozenProviderCollection, + only_build_tests: bool, ) -> anyhow::Result> { - Ok(match ::from_collection(providers) { - Some(provider) => { - if skip_build_based_on_labels(provider, label_filtering) { - return Ok(indexset![]); - } - let mut artifact_visitor = SimpleCommandLineArtifactVisitor::new(); - provider.visit_artifacts(&mut artifact_visitor)?; - artifact_visitor.inputs + 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![]); } - None => { - // not a test - indexset![] + let mut artifact_visitor = SimpleCommandLineArtifactVisitor::new(); + test_provider.visit_artifacts(&mut artifact_visitor)?; + artifacts.extend(artifact_visitor.inputs) + } + + if !only_build_tests { + if let Some(provider) = providers.builtin_provider::() { + provider.for_each_output(&mut |artifact| { + artifacts.insert(artifact); + })?; } - }) + } + Ok(artifacts) } - let artifacts_to_build = get_artifacts_to_build(label_filtering, providers)?; + + let artifacts_to_build = get_artifacts_to_build(label_filtering, providers, only_build_tests)?; // build the test target first ctx.try_compute_join(artifacts_to_build.iter(), |ctx, input| { ctx.ensure_artifact_group(input).boxed()