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 Jul 19, 2024
1 parent 37a44bb commit 268d222
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 25 deletions.
8 changes: 8 additions & 0 deletions app/buck2_cli_proto/daemon.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 32 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,36 @@ If include patterns are present, regardless of whether exclude patterns are pres
#[clap(long)]
test_executor_stderr: Option<OutputDestinationArg>,

#[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.
Expand Down Expand Up @@ -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),
Expand Down
88 changes: 63 additions & 25 deletions app/buck2_test/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -449,6 +454,8 @@ async fn test_targets(
missing_target_behavior: MissingTargetBehavior,
timeout: Option<Duration>,
ignore_tests_attribute: bool,
build_default_info: bool,
build_run_info: bool,
) -> anyhow::Result<TestOutcome> {
let session = Arc::new(session);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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![])
Expand Down Expand Up @@ -894,17 +907,26 @@ async fn test_target(
label_filtering: Arc<TestLabelFiltering>,
cell_resolver: &CellResolver,
working_dir_cell: CellName,
build_default_info: bool,
build_run_info: 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,
build_default_info,
build_run_info,
)
.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 @@ -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<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 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![]);
}
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::<FrozenDefaultInfo>() {
provider.for_each_output(&mut |artifact| {
artifacts.insert(artifact);
})?;
}
}

if build_run_info {
if let Some(provider) = providers.builtin_provider::<FrozenRunInfo>() {
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()
Expand Down

0 comments on commit 268d222

Please sign in to comment.