Skip to content

Commit

Permalink
Add a --build flag to buck2 test
Browse files Browse the repository at this point in the history
  • Loading branch information
cbarrete committed Jun 21, 2024
1 parent 350dada commit 42f6002
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 26 deletions.
3 changes: 3 additions & 0 deletions app/buck2_cli_proto/daemon.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions app/buck2_client/src/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ If include patterns are present, regardless of whether exclude patterns are pres
#[clap(long)]
test_executor_stderr: Option<OutputDestinationArg>,

#[clap(long)]
build: bool,

/// Additional arguments passed to the test executor.
///
/// Test executor is expected to have `--env` flag to pass environment variables.
Expand Down Expand Up @@ -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),
Expand Down
60 changes: 34 additions & 26 deletions app/buck2_test/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -373,6 +375,7 @@ async fn test(
MissingTargetBehavior::from_skip(build_opts.skip_missing_targets),
timeout,
request.ignore_tests_attribute,
only_build_tests,
)
.await?;

Expand Down Expand Up @@ -448,6 +451,7 @@ async fn test_targets(
missing_target_behavior: MissingTargetBehavior,
timeout: Option<Duration>,
ignore_tests_attribute: bool,
only_build_tests: bool,
) -> anyhow::Result<TestOutcome> {
let session = Arc::new(session);

Expand Down Expand Up @@ -531,6 +535,7 @@ async fn test_targets(
working_dir_cell,
missing_target_behavior,
ignore_tests_attribute,
only_build_tests,
});

driver.push_pattern(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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![])
Expand Down Expand Up @@ -893,17 +900,18 @@ async fn test_target(
label_filtering: Arc<TestLabelFiltering>,
cell_resolver: &CellResolver,
working_dir_cell: CellName,
only_build_tests: bool,
) -> anyhow::Result<Option<ConfiguredProvidersLabel>> {
// 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 <dyn TestProvider>::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(
Expand All @@ -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<IndexSet<ArtifactGroup>> {
Ok(match <dyn TestProvider>::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) = <dyn TestProvider>::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::<FrozenDefaultInfo>() {
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()
Expand Down

0 comments on commit 42f6002

Please sign in to comment.