Skip to content

Commit

Permalink
Fix incompatible target skipping for skylib's analysistest
Browse files Browse the repository at this point in the history
In bazelbuild#12366, Keith Smiley (@keith) reported that he was having trouble
using the new `target_compatible_with` attribute in `rules_swift`.
Specifically, setting it on an `analysistest` test target was giving
him the following error:

    ERROR: /workdir/test/BUILD:13:29: in coverage_xcode_prefix_map_test rule //test:coverage_settings_xcode_prefix_map: rules with analysis_test=true must return an instance of AnalysisTestResultInfo
    ERROR: Analysis of target '//test:coverage_settings_xcode_prefix_map' failed; build aborted: Analysis of target '//test:coverage_settings_xcode_prefix_map' failed

This was caused by the fact that for incompatible targets we create a
dummy `ConfiguredTarget` that only provides the bare minimum to make
the rest of bazel happy before it gets skipped. It turns out that
`analysistest` rules have additional checks that need to be satisfied.
This patch aims to satisfy those additional checks without impacting
functionality.

Fixes bazelbuild#12366
  • Loading branch information
philsc committed Oct 28, 2020
1 parent 9bdb4b1 commit 4596941
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironmentsProvider.RemovedEnvironmentCulprit;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.analysis.test.AnalysisTestResultInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand Down Expand Up @@ -968,6 +969,15 @@ private static ConfiguredTarget createIncompatibleConfiguredTarget(
"Both violatedConstraints and targetsResponsibleForIncompatibility are null");
}

// If this is an analysis test, RuleConfiguredTargetBuilder performs some additional sanity
// checks. Satisfy them with an appropriate provider.
if (ruleContext.getRule().isAnalysisTest()) {
builder.addNativeDeclaredProvider(
new AnalysisTestResultInfo(
/*success=*/ false,
"This test is incompatible and should not have been run. Please file a bug upstream."));
}

builder.add(RunfilesProvider.class, RunfilesProvider.simple(runfiles));
if (!outputArtifacts.isEmpty()) {
Artifact executable = outputArtifacts.get(0);
Expand Down
42 changes: 42 additions & 0 deletions src/test/shell/integration/target_compatible_with_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,48 @@ EOF
expect_log '^Hello World$'
}
# Validates that we successfully skip analysistest rule targets when they
# depend on incompatible targets.
function test_analysistest() {
setup_skylib_support
cat > target_skipping/analysistest.bzl <<EOF
load("@bazel_skylib//lib:unittest.bzl", "analysistest")
def _analysistest_test_impl(ctx):
env = analysistest.begin(ctx)
print("Running the test")
return analysistest.end(env)
analysistest_test = analysistest.make(_analysistest_test_impl)
EOF
cat >> target_skipping/BUILD <<EOF
load(":analysistest.bzl", "analysistest_test")
analysistest_test(
name = "foo3_analysistest_test",
target_under_test = ":some_foo3_target",
)
EOF
cd target_skipping || fail "couldn't cd into workspace"
bazel test --show_result=10 \
--host_platform=@//target_skipping:foo3_platform \
--platforms=@//target_skipping:foo3_platform \
//target_skipping:foo3_analysistest_test &> "${TEST_log}" \
|| fail "Bazel failed unexpectedly."
expect_log '^//target_skipping:foo3_analysistest_test * PASSED in'
bazel test --show_result=10 \
--host_platform=@//target_skipping:foo1_bar1_platform \
--platforms=@//target_skipping:foo1_bar1_platform \
//target_skipping:all &> "${TEST_log}" \
|| fail "Bazel failed unexpectedly."
expect_log '^Target //target_skipping:foo3_analysistest_test was skipped'
}
function write_query_test_targets() {
cat >> target_skipping/BUILD <<EOF
genrule(
Expand Down

0 comments on commit 4596941

Please sign in to comment.