Skip to content

Commit

Permalink
Flip trim_test_configuration default for Bazel.
Browse files Browse the repository at this point in the history
Referenced by #6842.

There is still a (rare) chance for breakage due to:
1. Trying to build a cc_test target with and without TestConfiguration under same build (leading to duplicate action errors)
2. A select statement that refers to a config_setting reading from TestConfiguration that is nested under a non-test rule.

RELNOTES: trim_test_configuration now defaults to on
PiperOrigin-RevId: 367695474
  • Loading branch information
twigg authored and copybara-github committed Apr 9, 2021
1 parent a41d418 commit ebac27e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,18 @@ public static class TestOptions extends FragmentOptions {
public int testResultExpiration;

@Option(
name = "trim_test_configuration",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
effectTags = {
OptionEffectTag.LOADING_AND_ANALYSIS,
OptionEffectTag.LOSES_INCREMENTAL_STATE,
},
help = "When enabled, test-related options will be cleared below the top level of the build. "
+ "When this flag is active, tests cannot be built as dependencies of non-test rules, "
+ "but changes to test-related options will not cause non-test rules to be re-analyzed."
)
name = "trim_test_configuration",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
effectTags = {
OptionEffectTag.LOADING_AND_ANALYSIS,
OptionEffectTag.LOSES_INCREMENTAL_STATE,
},
help =
"When enabled, test-related options will be cleared below the top level of the build."
+ " When this flag is active, tests cannot be built as dependencies of non-test"
+ " rules, but changes to test-related options will not cause non-test rules to be"
+ " re-analyzed.")
public boolean trimTestConfiguration;

@Option(
Expand Down
20 changes: 10 additions & 10 deletions src/test/shell/bazel/allowlist_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ function test_allowlist_includes_external_deps() {
cat > hotsauce/rules.bzl <<EOF
def _my_transition_impl(settings, attr):
_ignore = (settings, attr)
return {'//command_line_option:test_arg': ['tapatio']}
return {'//command_line_option:platform_suffix': 'tapatio'}
my_transition = transition(
implementation = _my_transition_impl,
inputs = [],
outputs = ["//command_line_option:test_arg"]
outputs = ["//command_line_option:platform_suffix"]
)
def _rule_with_transition_impl(ctx):
return []
Expand Down Expand Up @@ -95,11 +95,11 @@ rule_with_external_dep(
)
EOF

bazel cquery "deps(//vinegar)" --test_arg=hotlanta --transitions=full \
bazel cquery "deps(//vinegar)" --platform_suffix=hotlanta --transitions=full \
--noimplicit_deps \
>& $TEST_log || fail "failed to query //vinegar"
expect_log "@secret_ingredient//hotsauce"
expect_log "test_arg:\[hotlanta\] -> \[\[\"tapatio\"\]\]"
expect_log "platform_suffix:hotlanta -> \[tapatio\]"

}

Expand All @@ -118,11 +118,11 @@ function test_allowlist_bad_value() {
cat > vinegar/rules.bzl <<EOF
def _my_transition_impl(settings, attr):
_ignore = (settings, attr)
return {'//command_line_option:test_arg': ['tapatio']}
return {'//command_line_option:platform_suffix': 'tapatio'}
my_transition = transition(
implementation = _my_transition_impl,
inputs = [],
outputs = ["//command_line_option:test_arg"]
outputs = ["//command_line_option:platform_suffix"]
)
def _rule_with_transition_impl(ctx):
return []
Expand Down Expand Up @@ -177,11 +177,11 @@ EOF
cat > vinegar/rules.bzl <<EOF
def _my_transition_impl(settings, attr):
_ignore = (settings, attr)
return {'//command_line_option:test_arg': ['tapatio']}
return {'//command_line_option:platform_suffix': 'tapatio'}
my_transition = transition(
implementation = _my_transition_impl,
inputs = [],
outputs = ["//command_line_option:test_arg"]
outputs = ["//command_line_option:platform_suffix"]
)
def _rule_with_transition_impl(ctx):
return []
Expand All @@ -202,10 +202,10 @@ rule_with_transition(
rule_with_transition(name = "acetic-acid")
EOF

bazel cquery "deps(//vinegar)" --test_arg=hotlanta --transitions=full \
bazel cquery "deps(//vinegar)" --platform_suffix=hotlanta --transitions=full \
--noimplicit_deps \
>& $TEST_log || fail "failed to query //vinegar"
expect_log "test_arg:\[hotlanta\] -> \[\[\"tapatio\"\]\]"
expect_log "platform_suffix:hotlanta -> \[tapatio\]"
}

run_suite "allowlist tests"

0 comments on commit ebac27e

Please sign in to comment.