diff options
author | 2017-08-22 01:59:46 +0200 | |
---|---|---|
committer | 2017-08-22 09:15:29 +0200 | |
commit | 49b5bbc665b0cef3bcc9686cb2148e5a2d4ee7c6 (patch) | |
tree | 345a195a167f4fd3d5885ad4cec9c9fb57e6c275 /src/test/java/com/google/devtools/build | |
parent | e2eea44b6938bff56eafbd2a8d7beb751a925582 (diff) |
Remove FeaturePolicyConfiguration et al. in favor of the new Whitelisting.
This migrates the config_feature_flag implementation over and removes the
old flag (which was not used except to test it). Fare thee well, old flag.
RELNOTES: None.
PiperOrigin-RevId: 165995681
Diffstat (limited to 'src/test/java/com/google/devtools/build')
10 files changed, 42 insertions, 735 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index cd0ea1419f..7348bd6a1b 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -39,7 +39,6 @@ filegroup( "//src/test/java/com/google/devtools/build/lib/rules/android:srcs", "//src/test/java/com/google/devtools/build/lib/rules/apple:srcs", "//src/test/java/com/google/devtools/build/lib/rules/config:srcs", - "//src/test/java/com/google/devtools/build/lib/analysis/featurecontrol:srcs", "//src/test/java/com/google/devtools/build/lib/analysis/platform:srcs", "//src/test/java/com/google/devtools/build/lib/analysis/whitelisting:srcs", "//src/test/java/com/google/devtools/build/lib/rules/platform:srcs", @@ -366,7 +365,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", "//src/main/java/com/google/devtools/build/lib/query2", "//src/main/java/com/google/devtools/build/lib/query2:query-output", "//src/main/java/com/google/devtools/build/lib/rules/apple", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD deleted file mode 100644 index 2f26a91fd6..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD +++ /dev/null @@ -1,32 +0,0 @@ -# Description: -# Tests for configuration mechanisms for rolling out and deprecating pieces of Bazel functionality. - -licenses(["notice"]) # Apache 2.0 - -filegroup( - name = "srcs", - srcs = glob( - ["**"], - ), - visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"], -) - -java_test( - name = "FeatureRestrictionTests", - srcs = glob(["*.java"]), - test_class = "com.google.devtools.build.lib.AllTests", - deps = [ - "//src/main/java/com/google/devtools/build/lib:build-base", - "//src/main/java/com/google/devtools/build/lib:packages-internal", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", - "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/common/options", - "//src/test/java/com/google/devtools/build/lib:analysis_testutil", - "//src/test/java/com/google/devtools/build/lib:packages_testutil", - "//src/test/java/com/google/devtools/build/lib:test_runner", - "//third_party:guava", - "//third_party:guava-testlib", - "//third_party:junit4", - "//third_party:truth", - ], -) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java deleted file mode 100644 index eaaf438215..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.PackageSpecification; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyConfiguration. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyConfigurationTest { - - @Test - public void isFeatureEnabledForRule_FalseIfAbsentFromFeatureMap() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.<String, PackageSpecification>of(), - ImmutableMap.of("newFeature", "//package_group:empty")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule"))) - .isFalse(); - } - - @Test - public void isFeatureEnabledForRule_TrueIfMappedToEverything() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.of("newFeature", "//...")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule"))) - .isTrue(); - } - - @Test - public void isFeatureEnabledForRule_TrueIfInPackageSpecification() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of( - "newFeature", - PackageSpecification.fromString(RepositoryName.MAIN, "//allowed/...")), - ImmutableMap.of("newFeature", "//allowed/...")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//allowed:rule"))) - .isTrue(); - } - - @Test - public void isFeatureEnabledForRule_FalseIfNotInPackageSpecification() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of( - "newFeature", - PackageSpecification.fromString(RepositoryName.MAIN, "//allowed/...")), - ImmutableMap.of("newFeature", "//allowed/...")); - - assertThat( - config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//forbidden:rule"))) - .isFalse(); - } - - @Test - public void isFeatureEnabledForRule_FailsIfNotPresentInPolicyList() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of()); - - try { - config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule")); - fail("Expected an exception."); - } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessageThat().contains("No such feature: newFeature"); - } - } - - @Test - public void getPolicyForFeature_ReturnsValueFromPolicy() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of("newFeature", "my policy")); - - assertThat(config.getPolicyForFeature("newFeature")).isEqualTo("my policy"); - } - - @Test - public void getPolicyForFeature_FailsIfNotPresentInPolicyList() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of()); - - try { - config.getPolicyForFeature("newFeature"); - fail("Expected an exception."); - } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessageThat().contains("No such feature: newFeature"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java deleted file mode 100644 index a28752a0fd..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java +++ /dev/null @@ -1,322 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; -import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; -import com.google.devtools.build.lib.cmdline.Label; -import java.util.Collection; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyLoader. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyLoaderTest extends AnalysisTestCase { - - private FeaturePolicyConfiguration getFragmentWithFeatures( - Iterable<String> allowedFeatures, Collection<String> args) throws Exception { - ConfigurationEnvironment env = - new ConfigurationEnvironment.TargetProviderEnvironment( - skyframeExecutor.getPackageManager(), reporter, directories); - BuildOptions options = - BuildOptions.of( - ImmutableList.<Class<? extends FragmentOptions>>of(FeaturePolicyOptions.class), - args.toArray(new String[0])); - return (FeaturePolicyConfiguration) - new FeaturePolicyLoader(allowedFeatures).create(env, options); - } - - @Test - public void specifiedFeaturesGetListedAccessPolicy() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='featured',", - " packages=[", - " '//direct',", - " '//recursive/...',", - " ])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:featured")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:featured"); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:featured"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct:allow"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct/child:allow"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//recursive:allow"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//recursive/child:allow"))) - .isTrue(); - } - - @Test - public void resolvesIncludedPackageGroups() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='main',", - " packages=['//a'],", - " includes=[':include'])", - "package_group(", - " name='include',", - " packages=['//b'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:main")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:main"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void resolvesAliasToPolicy() throws Exception { - scratch.file( - "policy/BUILD", - "alias(", - " name='aliased',", - " actual=':featured')", - "package_group(", - " name='featured',", - " packages=[", - " '//direct',", - " '//recursive/...',", - " ])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:aliased")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:aliased"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:aliased"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:featured"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct:allow"))) - .isTrue(); - } - - @Test - public void resolvesAliasToIncludesInPackageGroups() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='main',", - " packages=['//a'],", - " includes=[':aliased'])", - "alias(", - " name='aliased',", - " actual=':include')", - "package_group(", - " name='include',", - " packages=['//b'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:main")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:main"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void allowsCyclesInPackageGroupsAndCoversAllMembersOfCycle() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='cycle',", - " packages=['//a'],", - " includes=[':elcyc'])", - "package_group(", - " name='elcyc',", - " packages=['//b'],", - " includes=[':cycle'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:cycle")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:cycle"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void unspecifiedFeaturesGetUniversalAccessPolicy() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//null:null")); - assertThat(fragment.getPolicyForFeature("defaultFeature")).isEqualTo("//..."); - assertThat(fragment.isFeatureEnabledForRule("defaultFeature", Label.parseAbsolute("//:rule"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "defaultFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isTrue(); - } - - @Test - public void throwsForFeatureWithMultiplePolicyDefinitions() throws Exception { - scratch.file( - "null/BUILD", - "package_group(name='null', packages=[])", - "package_group(name='empty', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("duplicateFeature"), - ImmutableList.of( - "--feature_control_policy=duplicateFeature=//null:null", - "--feature_control_policy=duplicateFeature=//null:empty")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("Multiple definitions"); - assertThat(expected).hasMessageThat().contains("duplicateFeature"); - } - } - - @Test - public void throwsForFeatureNotSpecifiedInLoader() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("otherFeature"), - ImmutableList.of("--feature_control_policy=missingFeature=//null:null")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("No such feature"); - assertThat(expected).hasMessageThat().contains("missingFeature"); - } - } - - @Test - public void throwsForFeatureWithNonexistentPolicy() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//null:missing")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("no such target '//null:missing'"); - } - } - - @Test - public void throwsForFeatureWithPolicyInNonexistentPackage() throws Exception { - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//missing:missing")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("no such package 'missing'"); - } - } - - @Test - public void throwsForFeatureWithNonPackageGroupPolicy() throws Exception { - scratch.file("policy/BUILD", "filegroup(name='non_package_group')"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//policy:non_package_group")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected) - .hasMessageThat() - .contains( - "//policy:non_package_group is not a package_group in brokenFeature feature policy"); - } - } - - @Test - public void throwsForFeatureWithNonRulePolicy() throws Exception { - scratch.file("policy/BUILD", "exports_files(['not_even_a_rule'])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//policy:not_even_a_rule")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected) - .hasMessageThat() - .contains( - "//policy:not_even_a_rule is not a package_group in brokenFeature feature policy"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java deleted file mode 100644 index 72f17c66d2..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.common.testing.EqualsTester; -import com.google.devtools.common.options.Options; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyOptions. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyOptionsTest { - - @Test - public void testEquality() throws Exception { - new EqualsTester() - .addEqualityGroup( - Options.getDefaults(FeaturePolicyOptions.class), - Options.getDefaults(FeaturePolicyOptions.class)) - .addEqualityGroup( - Options.parse( - FeaturePolicyOptions.class, - "--feature_control_policy=feature=//test:rest", - "--feature_control_policy=future=//nest:best") - .getOptions()) - .testEquals(); - } - - @Test - public void testHostVersionCopiesPolicies() throws Exception { - FeaturePolicyOptions base = - Options.parse( - FeaturePolicyOptions.class, - "--feature_control_policy=feature=//test:rest", - "--feature_control_policy=future=//nest:best") - .getOptions(); - FeaturePolicyOptions host = base.getHost(false); - FeaturePolicyOptions hostFallback = base.getHost(true); - assertThat(host.policies).containsExactlyElementsIn(base.policies); - assertThat(hostFallback.policies).containsExactlyElementsIn(base.policies); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java deleted file mode 100644 index d673450efb..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.common.options.OptionsParsingException; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the policy entry option converter. */ -@RunWith(JUnit4.class) -public final class PolicyEntryConverterTest { - - @Test - public void convert_successfullyConvertsValidPair() throws Exception { - assertThat(new PolicyEntryConverter().convert("jeepers=//creepers:peepers")) - .isEqualTo(PolicyEntry.create("jeepers", Label.parseAbsolute("//creepers:peepers"))); - } - - @Test - public void convert_successfullyConvertsValidLabelWithEqualsSign() throws Exception { - assertThat(new PolicyEntryConverter().convert("slamjam=//whoomp:it=there")) - .isEqualTo(PolicyEntry.create("slamjam", Label.parseAbsolute("//whoomp:it=there"))); - } - - @Test - public void convert_acceptsBarePackageNameAsDefaultTarget() throws Exception { - assertThat(new PolicyEntryConverter().convert("two=//go")) - .isEqualTo(PolicyEntry.create("two", Label.parseAbsolute("//go:go"))); - } - - @Test - public void convert_acceptsRepositoryPrefixedLabel() throws Exception { - assertThat(new PolicyEntryConverter().convert("here=@somewhere//else:where")) - .isEqualTo(PolicyEntry.create("here", Label.parseAbsolute("@somewhere//else:where"))); - } - - @Test - public void convert_acceptsBareRepository() throws Exception { - assertThat(new PolicyEntryConverter().convert("aliens=@space")) - .isEqualTo(PolicyEntry.create("aliens", Label.parseAbsolute("@space//:space"))); - } - - @Test - public void convert_failsToConvertWithoutDivider() throws Exception { - try { - new PolicyEntryConverter().convert("hack//sign:on"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertLabelAlone() throws Exception { - try { - new PolicyEntryConverter().convert("//plain:label"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertFeatureIdAlone() throws Exception { - try { - new PolicyEntryConverter().convert("plainFeature"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertEmptyFeature() throws Exception { - try { - new PolicyEntryConverter().convert("=//R1:C1"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("feature cannot be empty"); - } - } - - @Test - public void convert_failsToConvertEmptyLabel() throws Exception { - try { - new PolicyEntryConverter().convert("warp="); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("label cannot be empty"); - } - } - - @Test - public void convert_failsToConvertInvalidLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=//wrong:wrong//wrong"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("target names may not contain '//' path separators"); - } - } - - @Test - public void convert_failsToConvertRelativeLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=wrong:wrong"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("invalid label: wrong:wrong"); - } - } - - @Test - public void convert_failsToConvertFilesystemPathLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=/usr/local/google/tmp/filename.ext"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("invalid label: /usr/local/google/tmp/filename.ext"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index ad149eab70..96e6d17f7c 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -13,8 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.mock; -import static com.google.devtools.build.lib.rules.core.CoreRules.FEATURE_POLICY_FEATURES; - import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -22,7 +20,6 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.PlatformConfigurationLoader; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyLoader; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.bazel.rules.BazelConfiguration; import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration; @@ -139,6 +136,18 @@ public final class BazelAnalysisMock extends AnalysisMock { "exports_files(['precompile.py'])", "sh_binary(name='2to3', srcs=['2to3.sh'])"); + // Use an alias package group to allow for modification at the simpler path + config.create( + "/bazel_tools_workspace/tools/whitelists/config_feature_flag/BUILD", + "package_group(", + " name='config_feature_flag',", + " includes=['@//tools/whitelists/config_feature_flag'],", + ")"); + + config.create( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name='config_feature_flag', packages=['//...'])"); + config.create( "/bazel_tools_workspace/tools/zip/BUILD", "package(default_visibility=['//visibility:public'])", @@ -247,7 +256,6 @@ public final class BazelAnalysisMock extends AnalysisMock { new J2ObjcConfiguration.Loader(), new ProtoConfiguration.Loader(), new ConfigFeatureFlagConfiguration.Loader(), - new FeaturePolicyLoader(FEATURE_POLICY_FEATURES), new AndroidConfiguration.Loader(), new PlatformConfigurationLoader()); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 9295234d76..2eba0f6d52 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -3042,10 +3042,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { @Test public void testFeatureFlagPolicyMustContainRuleToUseFeatureFlags() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file( "flag/BUILD", @@ -3065,22 +3065,19 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " '//flag:flag': 'right',", " }", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNull(); assertContainsEvent( - "in android_binary rule //java/com/google/android/foo:foo: the feature_flags attribute is " - + "not available in package 'java/com/google/android/foo' according to policy " - + "'//policy:feature_flag_users'"); + "in feature_flags attribute of android_binary rule //java/com/google/android/foo:foo: " + + "the feature_flags attribute is not available in package " + + "'java/com/google/android/foo'"); } @Test public void testFeatureFlagPolicyDoesNotBlockRuleIfInPolicy() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag', '//java/com/google/android/foo'])"); scratch.file( "flag/BUILD", @@ -3100,19 +3097,16 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " '//flag:flag': 'right',", " }", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull(); assertNoEvents(); } @Test public void testFeatureFlagPolicyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file("flag/BUILD"); scratch.file( @@ -3122,9 +3116,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " manifest = 'AndroidManifest.xml',", " srcs = [':FooFlags.java'],", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull(); assertNoEvents(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java index c56ab884b4..2ee5755c40 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java @@ -291,9 +291,9 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { @Test public void policy_mustContainRulesPackage() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", - "package_group(name = 'feature_flag_users', packages = ['//some/other'])"); + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name = 'config_feature_flag', packages = ['//some/other'])"); scratch.file( "test/BUILD", "config_feature_flag(", @@ -301,20 +301,17 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { " allowed_values = ['default', 'configured', 'other'],", " default_value = 'default',", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag")).isNull(); assertContainsEvent( "in config_feature_flag rule //test:flag: the config_feature_flag rule is not available in " - + "package 'test' according to policy '//policy:feature_flag_users'"); + + "package 'test'"); } @Test public void policy_doesNotBlockRuleIfInPackageGroup() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(name = 'feature_flag_users', packages = ['//test'])"); + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name = 'config_feature_flag', packages = ['//test'])"); scratch.file( "test/BUILD", "config_feature_flag(", @@ -322,9 +319,6 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { " allowed_values = ['default', 'configured', 'other'],", " default_value = 'default',", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag")).isNotNull(); assertNoEvents(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java index b5935056a8..c79332b4f3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java @@ -1126,10 +1126,10 @@ public class ConfigSettingTest extends BuildViewTestCase { @Test public void policyMustContainRuleToUseFlagValues() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file( "flag/BUILD", @@ -1147,22 +1147,18 @@ public class ConfigSettingTest extends BuildViewTestCase { " '//flag:flag': 'right',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNull(); assertContainsEvent( - "in config_setting rule //test:flag_values_user: the flag_values attribute is not " - + "available in package 'test' according to policy " - + "'//policy:feature_flag_users'"); + "in flag_values attribute of config_setting rule //test:flag_values_user: the flag_values " + + "attribute is not available in package 'test'"); } @Test public void policyDoesNotBlockRuleIfInPolicy() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag', '//test'])"); scratch.file( "flag/BUILD", @@ -1180,19 +1176,16 @@ public class ConfigSettingTest extends BuildViewTestCase { " '//flag:flag': 'right',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull(); assertNoEvents(); } @Test public void policyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file("flag/BUILD"); scratch.file( @@ -1203,9 +1196,6 @@ public class ConfigSettingTest extends BuildViewTestCase { " 'cpu': 'k8',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull(); assertNoEvents(); } |