Unit tests respect top-level rule-class transitions.

--
PiperOrigin-RevId: 151129669
MOS_MIGRATED_REVID=151129669

3 files changed, 87 insertions, 37 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 6043cda299..3431013053 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -54,8 +54,12 @@ import com.google.devtools.build.lib.packages.AspectClass;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.PackageSpecification;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
@@ -1088,14 +1092,39 @@ public class BuildView {
     return result;
   }
 
+  private Transition getTopLevelTransitionForTarget(Label label, ExtendedEventHandler handler) {
+    Rule rule;
+    try {
+      rule = skyframeExecutor
+          .getPackageManager()
+          .getTarget(handler, label)
+          .getAssociatedRule();
+    } catch (NoSuchPackageException | NoSuchTargetException e) {
+      return ConfigurationTransition.NONE;
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new AssertionError("Configuration of " + label + " interrupted");
+    }
+    if (rule == null) {
+      return ConfigurationTransition.NONE;
+    }
+    RuleTransitionFactory factory = rule
+        .getRuleClassObject()
+        .getTransitionFactory();
+    return (factory == null) ? ConfigurationTransition.NONE : factory.buildTransitionFor(rule);
+  }
+
   /**
-   * Returns a configured target for the specified target and configuration. Returns {@code null} if
-   * something goes wrong.
+   * Returns a configured target for the specified target and configuration. If dynamic
+   * configurations are activated, and the target in question has a top-level rule class transition,
+   * that transition is applied in the returned ConfiguredTarget. Returns {@code null} if something
+   * goes wrong.
    */
   @VisibleForTesting
   public ConfiguredTarget getConfiguredTargetForTesting(
       ExtendedEventHandler eventHandler, Label label, BuildConfiguration config) {
-    return skyframeExecutor.getConfiguredTargetForTesting(eventHandler, label, config);
+    return skyframeExecutor.getConfiguredTargetForTesting(eventHandler, label, config,
+        getTopLevelTransitionForTarget(label, eventHandler));
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 2dc660ff68..c6d088d29d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -82,6 +82,7 @@ import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.exec.OutputService;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
@@ -1442,13 +1443,32 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
   /**
    * Returns a particular configured target.
-   *
-   * <p>Used only for testing.
    */
   @VisibleForTesting
   @Nullable
   public ConfiguredTarget getConfiguredTargetForTesting(
       ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration) {
+    return getConfiguredTargetForTesting(
+        eventHandler, label, configuration, ConfigurationTransition.NONE);
+  }
+
+  /**
+   * Returns a particular configured target. If dynamic configurations are active, applies the given
+   * configuration transition.
+   */
+  @VisibleForTesting
+  @Nullable
+  public ConfiguredTarget getConfiguredTargetForTesting(
+      ExtendedEventHandler eventHandler,
+      Label label,
+      BuildConfiguration configuration,
+      Attribute.Transition transition) {
+
+    Preconditions.checkArgument(configuration == null
+        || configuration.useDynamicConfigurations()
+        || transition == ConfigurationTransition.NONE,
+        "Dynamic configurations required for test configuration using a transition");
+
     if (memoizingEvaluator.getExistingValueForTesting(
         PrecomputedValue.WORKSPACE_STATUS_KEY.getKeyForTesting()) == null) {
       injectWorkspaceStatusData(label.getWorkspaceRoot());
@@ -1458,8 +1478,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     if (configuration == null) {
       dep = Dependency.withNullConfiguration(label);
     } else if (configuration.useDynamicConfigurations()) {
-      dep = Dependency.withTransitionAndAspects(label, Attribute.ConfigurationTransition.NONE,
-          AspectCollection.EMPTY);
+      dep = Dependency.withTransitionAndAspects(label, transition, AspectCollection.EMPTY);
     } else {
       dep = Dependency.withConfiguration(label, configuration);
     }
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 20e767b1b2..e8271372d2 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
@@ -677,7 +677,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
 
   /**
    * Returns the ConfiguredTarget for the specified label, configured for the "build" (aka "target")
-   * configuration.
+   * configuration. If the label corresponds to a target with a top-level configuration transition,
+   * that transition is applied to the given config in the returned ConfiguredTarget.
    */
   public ConfiguredTarget getConfiguredTarget(String label)
       throws LabelSyntaxException {
@@ -685,8 +686,9 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
   }
 
   /**
-   * Returns the ConfiguredTarget for the specified label, using the
-   * given build configuration.
+   * Returns the ConfiguredTarget for the specified label, using the given build configuration. If
+   * the label corresponds to a target with a top-level configuration transition, that transition is
+   * applied to the given config in the returned ConfiguredTarget.
    */
   protected ConfiguredTarget getConfiguredTarget(String label, BuildConfiguration config)
       throws LabelSyntaxException {
@@ -695,7 +697,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
 
   /**
    * Returns the ConfiguredTarget for the specified label, using the
-   * given build configuration.
+   * given build configuration. If the label corresponds to a target with a top-level configuration
+   * transition, that transition is applied to the given config in the returned ConfiguredTarget.
    *
    * <p>If the evaluation of the SkyKey corresponding to the configured target fails, this
    * method may return null.  In that case, use a debugger to inspect the {@link ErrorInfo}
@@ -704,8 +707,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
    * {@link SkyframeExecutor#getConfiguredTargetForTesting}.  See also b/26382502.
    */
   protected ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration config) {
-    return view.getConfiguredTargetForTesting(
-        reporter, BlazeTestUtils.convertLabel(label), config);
+    return view.getConfiguredTargetForTesting(reporter, BlazeTestUtils.convertLabel(label), config);
   }
 
   /**
@@ -969,7 +971,11 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
    * be "foo.o".
    */
   protected Artifact getBinArtifact(String packageRelativePath, String owner) {
-    return getBinArtifact(packageRelativePath, makeLabelAndConfiguration(owner));
+    ConfiguredTargetKey config = makeLabelAndConfiguration(owner);
+    return getPackageRelativeDerivedArtifact(
+        packageRelativePath,
+        config.getConfiguration().getBinDirectory(RepositoryName.MAIN),
+        config);
   }
 
   /**
@@ -1025,18 +1031,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
   }
 
   /**
-   * Gets a derived Artifact for testing in the subdirectory of the {@link
-   * BuildConfiguration#getBinDirectory} corresponding to the package of {@code owner}. So
-   * to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just
-   * be "foo.o".
-   */
-  private Artifact getBinArtifact(String packageRelativePath, ArtifactOwner owner) {
-    return getPackageRelativeDerivedArtifact(packageRelativePath,
-        targetConfig.getBinDirectory(RepositoryName.MAIN),
-        owner);
-  }
-
-  /**
    * Gets a derived Artifact for testing in the {@link BuildConfiguration#getGenfilesDirectory}.
    * This method should only be used for tests that do no analysis, and so there is no
    * ConfiguredTarget to own this artifact. If the test runs the analysis phase, {@link
@@ -1055,7 +1049,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
    * just be "foo.o".
    */
   protected Artifact getGenfilesArtifact(String packageRelativePath, String owner) {
-    return getGenfilesArtifact(packageRelativePath, makeLabelAndConfiguration(owner));
+    ConfiguredTargetKey configKey = makeLabelAndConfiguration(owner);
+    return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
   }
 
   /**
@@ -1065,7 +1060,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
    * just be "foo.o".
    */
   protected Artifact getGenfilesArtifact(String packageRelativePath, ConfiguredTarget owner) {
-    return getGenfilesArtifact(packageRelativePath, new ConfiguredTargetKey(owner));
+    ConfiguredTargetKey configKey = new ConfiguredTargetKey(owner);
+    return getGenfilesArtifact(packageRelativePath, configKey, configKey.getConfiguration());
   }
 
   /**
@@ -1114,14 +1110,14 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
 
   /**
    * Gets a derived Artifact for testing in the subdirectory of the {@link
-   * BuildConfiguration#getGenfilesDirectory} corresponding to the package of {@code owner}.
-   * So to specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should
-   * just be "foo.o".
+   * BuildConfiguration#getGenfilesDirectory} corresponding to the package of {@code owner}. So to
+   * specify a file foo/foo.o owned by target //foo:foo, {@code packageRelativePath} should just be
+   * "foo.o".
    */
-  private Artifact getGenfilesArtifact(String packageRelativePath, ArtifactOwner owner) {
-    return getPackageRelativeDerivedArtifact(packageRelativePath,
-        targetConfig.getGenfilesDirectory(RepositoryName.MAIN),
-        owner);
+  private Artifact getGenfilesArtifact(
+      String packageRelativePath, ArtifactOwner owner, BuildConfiguration config) {
+    return getPackageRelativeDerivedArtifact(
+        packageRelativePath, config.getGenfilesDirectory(RepositoryName.MAIN), owner);
   }
 
   /**
@@ -1246,11 +1242,17 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
   }
 
   private ConfiguredTargetKey makeLabelAndConfiguration(String label) {
-    BuildConfiguration config = targetConfig;
+    BuildConfiguration config;
+    try {
+      config = getConfiguredTarget(label).getConfiguration();
+    } catch (LabelSyntaxException e) {
+      throw new IllegalArgumentException(e);
+    }
     if (targetConfig.useDynamicConfigurations()) {
       try {
-        config = view.getDynamicConfigurationForTesting(getTarget(label), targetConfig, reporter);
+        config = view.getDynamicConfigurationForTesting(getTarget(label), config, reporter);
       } catch (Exception e) {
+        //TODO(b/36585204): Clean this up
         throw new RuntimeException(e);
       }
     }