diff options
Diffstat (limited to 'src/test/java/com/google/devtools')
8 files changed, 74 insertions, 141 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java index 00130f8282..1a7e353385 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java @@ -34,7 +34,7 @@ import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.util.BuildViewTestBase; -import com.google.devtools.build.lib.analysis.util.ExpectedDynamicConfigurationErrors; +import com.google.devtools.build.lib.analysis.util.ExpectedTrimmedConfigurationErrors; import com.google.devtools.build.lib.analysis.util.MockRule; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; @@ -527,10 +527,9 @@ public class BuildViewTest extends BuildViewTestBase { assertContainsEvent("and referenced by '//foo:bad'"); assertContainsEvent("in sh_library rule //foo"); assertContainsEvent("cycle in dependency graph"); - // Dynamic configurations trigger this error both in configuration trimming (which visits - // the transitive target closure) and in the normal configured target cycle detection path. - // So we get an additional instance of this check (which varies depending on whether Skyframe - // loading phase is enabled). + // This error is triggered both in configuration trimming (which visits the transitive target + // closure) and in the normal configured target cycle detection path. So we get an additional + // instance of this check (which varies depending on whether Skyframe loading phase is enabled). // TODO(gregce): Fix above and uncomment the below. Note that the duplicate doesn't make it into // real user output (it only affects tests). // assertEventCount(3, eventCollector); @@ -924,10 +923,9 @@ public class BuildViewTest extends BuildViewTestBase { @Test public void testCycleDueToJavaLauncherConfiguration() throws Exception { - if (defaultFlags().contains(Flag.DYNAMIC_CONFIGURATIONS)) { - // Dynamic configurations don't yet support late-bound attributes. Development testing already - // runs all tests with dynamic configurations enabled, so this will still fail for developers - // and won't get lost in the fog. + if (defaultFlags().contains(Flag.TRIMMED_CONFIGURATIONS)) { + // Trimmed configurations don't yet support late-bound attributes. + // TODO(gregce): re-enable this when ready. return; } scratch.file("foo/BUILD", @@ -936,7 +934,7 @@ public class BuildViewTest extends BuildViewTestBase { // Everything is fine - the dependency graph is acyclic. update("//foo:java", "//foo:cpp"); if (getTargetConfiguration().trimConfigurations()) { - fail(ExpectedDynamicConfigurationErrors.LATE_BOUND_ATTRIBUTES_UNSUPPORTED); + fail(ExpectedTrimmedConfigurationErrors.LATE_BOUND_ATTRIBUTES_UNSUPPORTED); } // Now there will be an analysis-phase cycle because the java_binary now has an implicit dep on // the cc_binary launcher. @@ -1231,7 +1229,7 @@ public class BuildViewTest extends BuildViewTestBase { } @Test - public void testTopLevelTargetsAreTrimmedWithDynamicConfigurations() throws Exception { + public void testTopLevelTargetsAreTrimmedWithTrimmedConfigurations() throws Exception { scratch.file("foo/BUILD", "sh_library(name='x', ", " srcs=['x.sh'])"); @@ -1389,13 +1387,13 @@ public class BuildViewTest extends BuildViewTestBase { } } - /** Runs the same test with dynamic configurations. */ + /** Runs the same test with trimmed configurations. */ @TestSpec(size = Suite.SMALL_TESTS) @RunWith(JUnit4.class) - public static class WithDynamicConfigurations extends BuildViewTest { + public static class WithTrimmedConfigurations extends BuildViewTest { @Override protected FlagBuilder defaultFlags() { - return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS); + return super.defaultFlags().with(Flag.TRIMMED_CONFIGURATIONS); } } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java index af37c8b78f..11e04e9537 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java @@ -205,23 +205,13 @@ public class DependencyResolverTest extends AnalysisTestCase { assertThat(dep.getConfiguration()).isNull(); } - /** Runs the same test with trimmed dynamic configurations. */ + /** Runs the same test with trimmed configurations. */ @TestSpec(size = Suite.SMALL_TESTS) @RunWith(JUnit4.class) - public static class WithDynamicConfigurations extends DependencyResolverTest { + public static class WithTrimmedConfigurations extends DependencyResolverTest { @Override protected FlagBuilder defaultFlags() { - return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS); - } - } - - /** Runs the same test with untrimmed dynamic configurations. */ - @TestSpec(size = Suite.SMALL_TESTS) - @RunWith(JUnit4.class) - public static class WithDynamicConfigurationsNoTrim extends DependencyResolverTest { - @Override - protected FlagBuilder defaultFlags() { - return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS_NOTRIM); + return super.defaultFlags().with(Flag.TRIMMED_CONFIGURATIONS); } } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index 2235d0bb8b..0cdc4a3975 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -96,10 +96,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase { public enum Flag { KEEP_GOING, SKYFRAME_LOADING_PHASE, - // Dynamic configurations that only include the fragments a target needs to properly analyze. - DYNAMIC_CONFIGURATIONS, - // Dynamic configurations that always include all fragments even when targets don't need them. - DYNAMIC_CONFIGURATIONS_NOTRIM + // Configurations that only include the fragments a target needs to properly analyze. + TRIMMED_CONFIGURATIONS } /** Helper class to make it easy to enable and disable flags. */ @@ -227,10 +225,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase { ruleClassProvider.getConfigurationOptions())); optionsParser.parse(new String[] {"--default_visibility=public" }); optionsParser.parse(args); - if (defaultFlags().contains(Flag.DYNAMIC_CONFIGURATIONS)) { + if (defaultFlags().contains(Flag.TRIMMED_CONFIGURATIONS)) { optionsParser.parse("--experimental_dynamic_configs=on"); - } else if (defaultFlags().contains(Flag.DYNAMIC_CONFIGURATIONS_NOTRIM)) { - optionsParser.parse("--experimental_dynamic_configs=notrim"); } InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer(); @@ -264,30 +260,10 @@ public abstract class AnalysisTestCase extends FoundationTestCase { /** * Returns the target configuration for the most recent build, as created in Blaze's - * master configuration creation phase. Most significantly, this is never a dynamic - * configuration. + * master configuration creation phase. */ protected BuildConfiguration getTargetConfiguration() throws InterruptedException { - return getTargetConfiguration(false); - } - - /** - * Returns the target configuration for the most recent build. If useDynamicVersionIfEnabled is - * true and dynamic configurations are enabled, returns the dynamic version. Else returns the - * static version. - */ - // TODO(gregce): force getTargetConfiguration() to getTargetConfiguration(true) once we know - // all callers can handle the dynamic version - protected BuildConfiguration getTargetConfiguration(boolean useDynamicVersionIfEnabled) - throws InterruptedException { - BuildConfiguration targetConfig = - Iterables.getOnlyElement(masterConfig.getTargetConfigurations()); - if (useDynamicVersionIfEnabled) { - return skyframeExecutor.getConfigurationForTesting( - reporter, targetConfig.fragmentClasses(), targetConfig.getOptions()); - } else { - return targetConfig; - } + return Iterables.getOnlyElement(masterConfig.getTargetConfigurations()); } protected BuildConfiguration getHostConfiguration() { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 7c9fea4311..3d0b1d2317 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -75,7 +75,7 @@ import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey; import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.DynamicConfigsMode; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.ConfigsMode; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.PatchTransition; @@ -182,7 +182,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { protected BuildConfigurationCollection masterConfig; protected BuildConfiguration targetConfig; // "target" or "build" config private List<String> configurationArgs; - private DynamicConfigsMode dynamicConfigsMode = DynamicConfigsMode.NOTRIM; + private ConfigsMode configsMode = ConfigsMode.NOTRIM; protected OptionsParser optionsParser; private PackageCacheOptions packageCacheOptions; @@ -414,12 +414,12 @@ public abstract class BuildViewTestCase extends FoundationTestCase { ModifiedFileSet.EVERYTHING_MODIFIED, rootDirectory); if (alsoConfigs) { try { - // Also invalidate all configurations. This is important for dynamic configurations: by - // invalidating all files we invalidate CROSSTOOL, which invalidates CppConfiguration (and - // a few other fragments). So we need to invalidate the - // {@link SkyframeBuildView#hostConfigurationCache} as well. Otherwise we end up - // with old CppConfiguration instances. Even though they're logically equal to the new ones, - // CppConfiguration has no .equals() method and some production code expects equality. + // Also invalidate all configurations. This is important: by invalidating all files we + // invalidate CROSSTOOL, which invalidates CppConfiguration (and a few other fragments). So + // we need to invalidate the {@link SkyframeBuildView#hostConfigurationCache} as well. + // Otherwise we end up with old CppConfiguration instances. Even though they're logically + // equal to the new ones, CppConfiguration has no .equals() method and some production code + // expects equality. useConfiguration(configurationArgs.toArray(new String[0])); } catch (Exception e) { // There are enough dependers on this method that don't handle Exception that just passing @@ -441,7 +441,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { String[] actualArgs; actualArgs = Arrays.copyOf(args, args.length + 1); actualArgs[args.length] = "--experimental_dynamic_configs=" - + dynamicConfigsMode.toString().toLowerCase(); + + configsMode.toString().toLowerCase(); masterConfig = createConfigurations(actualArgs); targetConfig = getTargetConfiguration(); configurationArgs = Arrays.asList(actualArgs); @@ -449,11 +449,11 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** - * Makes subsequent {@link #useConfiguration} calls automatically enable dynamic configurations - * in the specified mode. + * Makes subsequent {@link #useConfiguration} calls automatically use the specified style for + * configurations. */ - protected final void useDynamicConfigurations(DynamicConfigsMode mode) { - dynamicConfigsMode = mode; + protected final void useConfigurationMode(ConfigsMode mode) { + configsMode = mode; } /** @@ -510,33 +510,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** - * Asserts that a target's prerequisites contain the given dependency. - */ - // TODO(bazel-team): replace this method with assertThat(iterable).contains(target). - // That doesn't work now because dynamic configurations aren't yet applied to top-level targets. - // This means that getConfiguredTarget("//go:two") returns a different configuration than - // requesting "//go:two" as a dependency. So the configured targets aren't considered "equal". - // Once we apply dynamic configs to top-level targets this discrepancy will go away. - protected void assertDirectPrerequisitesContain(ConfiguredTarget target, ConfiguredTarget dep) - throws Exception { - Iterable<ConfiguredTarget> prereqs = getDirectPrerequisites(target); - BuildConfiguration depConfig = dep.getConfiguration(); - for (ConfiguredTarget contained : prereqs) { - if (contained.getLabel().equals(dep.getLabel())) { - BuildConfiguration containedConfig = contained.getConfiguration(); - if (containedConfig == null && depConfig == null) { - return; - } else if (containedConfig != null - && depConfig != null - && containedConfig.cloneOptions().equals(depConfig.cloneOptions())) { - return; - } - } - } - fail("Cannot find " + target.toString() + " in " + prereqs.toString()); - } - - /** * Asserts that two configurations are the same. * * <p>Historically this meant they contained the same object reference. But with upcoming dynamic @@ -1154,12 +1127,12 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** - * Strips the C++-contributed prefix out of an output path when tests are run with dynamic + * Strips the C++-contributed prefix out of an output path when tests are run with trimmed * configurations. e.g. turns "bazel-out/gcc-X-glibc-Y-k8-fastbuild/ to "bazel-out/fastbuild/". * * <p>This should be used for targets use configurations with C++ fragments. */ - protected String stripCppPrefixForDynamicConfigs(String outputPath) { + protected String stripCppPrefixForTrimmedConfigs(String outputPath) { return targetConfig.trimConfigurations() ? AnalysisTestUtil.OUTPUT_PATH_CPP_PREFIX_PATTERN.matcher(outputPath).replaceFirst("") : outputPath; @@ -1302,7 +1275,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { BuildConfiguration config; try { config = getConfiguredTarget(label).getConfiguration(); - config = view.getDynamicConfigurationForTesting(getTarget(label), config, reporter); + config = view.getConfigurationForTesting(getTarget(label), config, reporter); } catch (LabelSyntaxException e) { throw new IllegalArgumentException(e); } catch (Exception e) { @@ -1497,8 +1470,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { } /** - * Returns the configuration created by applying the given transition to the source - * configuration. Works for both static and dynamic configuration tests. + * Returns the configuration created by applying the given transition to the source configuration. */ protected BuildConfiguration getConfiguration(BuildConfiguration fromConfig, Attribute.Transition transition) throws InterruptedException { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ExpectedDynamicConfigurationErrors.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ExpectedTrimmedConfigurationErrors.java index fb9fecf174..ca1c0bb5e6 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ExpectedDynamicConfigurationErrors.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ExpectedTrimmedConfigurationErrors.java @@ -14,12 +14,16 @@ package com.google.devtools.build.lib.analysis.util; /** - * Reference point for expected test failures when dynamic configurations are enabled. + * Reference point for expected test failures when trimmed configurations are enabled. * - * <p>Every Bazel test should either succeed with --experimental_dynamic_configs or + * <p>Every Bazel test should either succeed with --experimental_dynamic_configs=on or * fail with a clear reason due to known features gaps. */ -public class ExpectedDynamicConfigurationErrors { +public class ExpectedTrimmedConfigurationErrors { public static final String LATE_BOUND_ATTRIBUTES_UNSUPPORTED = - "dynamic configurations don't yet support fragments from late-bound dependencies"; + "trimmed configurations don't yet support fragments from late-bound dependencies"; + + public static final String MAKE_VARIABLE_FRAGMENTS_UNSUPPORTED = + "Trimmed configurations don't yet support fragments required by make variables. See " + + "b/25768144"; } 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 fbe81326ea..a856424046 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 @@ -37,7 +37,7 @@ import org.junit.runners.JUnit4; public final class ConfigFeatureFlagTest extends SkylarkTestCase { @Before - public void useDynamicConfigurations() throws Exception { + public void useTrimmedConfigurations() throws Exception { useConfiguration("--experimental_dynamic_configs=on"); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java index 10ad309133..edba22433d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java @@ -290,39 +290,32 @@ public class ConfigurationsForTargetsTest extends AnalysisTestCase { assertThat(toolDep.getConfiguration().isHostConfiguration()).isTrue(); } - /** Runs the same test with untrimmed dynamic configurations. */ - @TestSpec(size = Suite.SMALL_TESTS) - @RunWith(JUnit4.class) - public static class WithDynamicConfigurationsNoTrim extends ConfigurationsForTargetsTest { - @Override - protected FlagBuilder defaultFlags() { - return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS_NOTRIM); - } - + @Test + public void splitDeps() throws Exception { // This test does not pass with trimming because android_binary applies an aspect and aspects // are not yet correctly supported with trimming. - @Test - public void splitDeps() throws Exception { - scratch.file( - "java/a/BUILD", - "cc_library(name = 'lib', srcs = ['lib.cc'])", - "android_binary(name='a', manifest = 'AndroidManifest.xml', deps = [':lib'])"); - useConfiguration("--fat_apk_cpu=k8,armeabi-v7a"); - List<ConfiguredTarget> deps = getConfiguredDeps("//java/a:a", "deps"); - assertThat(deps).hasSize(2); - ConfiguredTarget dep1 = deps.get(0); - ConfiguredTarget dep2 = deps.get(1); - assertThat( - ImmutableList.<String>of( - dep1.getConfiguration().getCpu(), - dep2.getConfiguration().getCpu())) - .containsExactly("armeabi-v7a", "k8"); - // We don't care what order split deps are listed, but it must be deterministic. - assertThat( - ConfiguredTargetFunction.DYNAMIC_SPLIT_DEP_ORDERING.compare( - Dependency.withConfiguration(dep1.getLabel(), dep1.getConfiguration()), - Dependency.withConfiguration(dep2.getLabel(), dep2.getConfiguration()))) - .isLessThan(0); + if (defaultFlags().contains(Flag.TRIMMED_CONFIGURATIONS)) { + return; } + scratch.file( + "java/a/BUILD", + "cc_library(name = 'lib', srcs = ['lib.cc'])", + "android_binary(name='a', manifest = 'AndroidManifest.xml', deps = [':lib'])"); + useConfiguration("--fat_apk_cpu=k8,armeabi-v7a"); + List<ConfiguredTarget> deps = getConfiguredDeps("//java/a:a", "deps"); + assertThat(deps).hasSize(2); + ConfiguredTarget dep1 = deps.get(0); + ConfiguredTarget dep2 = deps.get(1); + assertThat( + ImmutableList.<String>of( + dep1.getConfiguration().getCpu(), + dep2.getConfiguration().getCpu())) + .containsExactly("armeabi-v7a", "k8"); + // We don't care what order split deps are listed, but it must be deterministic. + assertThat( + ConfiguredTargetFunction.DYNAMIC_SPLIT_DEP_ORDERING.compare( + Dependency.withConfiguration(dep1.getLabel(), dep1.getConfiguration()), + Dependency.withConfiguration(dep2.getLabel(), dep2.getConfiguration()))) + .isLessThan(0); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java index 54d8e55d02..d398c10427 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithDynamicConfigurationsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsWithTrimmedConfigurationsTest.java @@ -52,10 +52,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Runs an expanded set of ConfigurationsForTargetsTest with trimmed dynamic configurations. */ +/** Runs an expanded set of ConfigurationsForTargetsTest with trimmed configurations. */ @TestSpec(size = Suite.SMALL_TESTS) @RunWith(JUnit4.class) -public class ConfigurationsForTargetsWithDynamicConfigurationsTest +public class ConfigurationsForTargetsWithTrimmedConfigurationsTest extends ConfigurationsForTargetsTest { private ConfigurationResolver configResolver; @@ -67,7 +67,7 @@ public class ConfigurationsForTargetsWithDynamicConfigurationsTest @Override protected FlagBuilder defaultFlags() { - return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS); + return super.defaultFlags().with(Flag.TRIMMED_CONFIGURATIONS); } private static class EmptySplitTransition implements SplitTransition<BuildOptions> { @@ -430,7 +430,7 @@ public class ConfigurationsForTargetsWithDynamicConfigurationsTest ConfiguredTarget target = getView().getConfiguredTargetForTesting( reporter, Label.parseAbsoluteUnchecked("@//a:factory"), - getTargetConfiguration(true)); + getTargetConfiguration()); assertThat(target.getConfiguration().getFragment(TestConfiguration.class).getTestFilter()) .isEqualTo("SET ON COMMAND LINE: original and best"); } |