Add some testing methods to get ConfiguredTargetAndData more easily, …

…and rename some methods that were still called ConfiguredTargetAndTarget. Move some tests over to using ConfiguredTargetAndData instead of calling ConfiguredTarget#getConfiguration() directly.

PiperOrigin-RevId: 189642264

src/main/java/com/google/devtools/build/lib/analysis/BuildView.java

@@ -954,22 +954,59 @@ protected final void pollInterruptedStatus() throws InterruptedException {
 
   // For testing
   @VisibleForTesting
-  public Iterable<ConfiguredTarget> getDirectPrerequisitesForTesting(
-      ExtendedEventHandler eventHandler, ConfiguredTarget ct,
+  public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
+      ExtendedEventHandler eventHandler,
+      ConfiguredTarget ct,
       BuildConfigurationCollection configurations)
-      throws EvalException, InvalidConfigurationException,
-      InterruptedException, InconsistentAspectOrderException {
+      throws EvalException, InvalidConfigurationException, InterruptedException,
+          InconsistentAspectOrderException {
     return Collections2.transform(
-        skyframeExecutor.getConfiguredTargetsForTesting(
-            eventHandler,
-            ct.getConfiguration(),
-            ImmutableSet.copyOf(
-                getDirectPrerequisiteDependenciesForTesting(
-                        eventHandler, ct, configurations, /*toolchainContext=*/ null)
-                    .values())),
+        getConfiguredTargetAndDataDirectPrerequisitesForTesting(eventHandler, ct, configurations),
         ConfiguredTargetAndData::getConfiguredTarget);
   }
 
+  // TODO(janakr): pass the configuration in as a parameter here and above.
+  @VisibleForTesting
+  public Collection<ConfiguredTargetAndData>
+      getConfiguredTargetAndDataDirectPrerequisitesForTesting(
+          ExtendedEventHandler eventHandler,
+          ConfiguredTarget ct,
+          BuildConfigurationCollection configurations)
+          throws EvalException, InvalidConfigurationException, InterruptedException,
+              InconsistentAspectOrderException {
+    return getConfiguredTargetAndDataDirectPrerequisitesForTesting(
+        eventHandler, ct, ct.getConfiguration(), configurations);
+  }
+
+  @VisibleForTesting
+  public Collection<ConfiguredTargetAndData>
+      getConfiguredTargetAndDataDirectPrerequisitesForTesting(
+          ExtendedEventHandler eventHandler,
+          ConfiguredTargetAndData ct,
+          BuildConfigurationCollection configurations)
+          throws EvalException, InvalidConfigurationException, InterruptedException,
+              InconsistentAspectOrderException {
+    return getConfiguredTargetAndDataDirectPrerequisitesForTesting(
+        eventHandler, ct.getConfiguredTarget(), ct.getConfiguration(), configurations);
+  }
+
+  private Collection<ConfiguredTargetAndData>
+      getConfiguredTargetAndDataDirectPrerequisitesForTesting(
+          ExtendedEventHandler eventHandler,
+          ConfiguredTarget ct,
+          BuildConfiguration configuration,
+          BuildConfigurationCollection configurations)
+          throws EvalException, InvalidConfigurationException, InterruptedException,
+              InconsistentAspectOrderException {
+    return skyframeExecutor.getConfiguredTargetsForTesting(
+        eventHandler,
+        configuration,
+        ImmutableSet.copyOf(
+            getDirectPrerequisiteDependenciesForTesting(
+                    eventHandler, ct, configurations, /*toolchainContext=*/ null)
+                .values()));
+  }
+
   @VisibleForTesting
   public OrderedSetMultimap<Attribute, Dependency> getDirectPrerequisiteDependenciesForTesting(
       final ExtendedEventHandler eventHandler,
@@ -1146,9 +1183,9 @@ public ConfiguredTarget getConfiguredTargetForTesting(
   }
 
   @VisibleForTesting
-  public ConfiguredTargetAndData getConfiguredTargetAndTargetForTesting(
+  public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
       ExtendedEventHandler eventHandler, Label label, BuildConfiguration config) {
-    return skyframeExecutor.getConfiguredTargetAndTargetForTesting(eventHandler, label, config);
+    return skyframeExecutor.getConfiguredTargetAndDataForTesting(eventHandler, label, config);
   }
 
   /**

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -1665,36 +1665,34 @@ public ConfiguredTarget getConfiguredTargetForTesting(
       BuildConfiguration configuration,
       ConfigurationTransition transition) {
     ConfiguredTargetAndData configuredTargetAndData =
-        getConfiguredTargetAndTargetForTesting(eventHandler, label, configuration, transition);
+        getConfiguredTargetAndDataForTesting(eventHandler, label, configuration, transition);
     return configuredTargetAndData == null ? null : configuredTargetAndData.getConfiguredTarget();
   }
 
   @VisibleForTesting
   @Nullable
-  public ConfiguredTargetAndData getConfiguredTargetAndTargetForTesting(
+  public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
       ExtendedEventHandler eventHandler,
       Label label,
       BuildConfiguration configuration,
       ConfigurationTransition transition) {
-    ConfiguredTargetAndData configuredTargetAndData =
-        Iterables.getFirst(
-            getConfiguredTargetsForTesting(
-                eventHandler,
-                configuration,
-                ImmutableList.of(
-                    configuration == null
-                        ? Dependency.withNullConfiguration(label)
-                        : Dependency.withTransitionAndAspects(
-                            label, transition, AspectCollection.EMPTY))),
-            null);
-    return configuredTargetAndData;
+    return Iterables.getFirst(
+        getConfiguredTargetsForTesting(
+            eventHandler,
+            configuration,
+            ImmutableList.of(
+                configuration == null
+                    ? Dependency.withNullConfiguration(label)
+                    : Dependency.withTransitionAndAspects(
+                        label, transition, AspectCollection.EMPTY))),
+        null);
   }
 
   @VisibleForTesting
   @Nullable
-  public ConfiguredTargetAndData getConfiguredTargetAndTargetForTesting(
+  public ConfiguredTargetAndData getConfiguredTargetAndDataForTesting(
       ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration) {
-    return getConfiguredTargetAndTargetForTesting(
+    return getConfiguredTargetAndDataForTesting(
         eventHandler, label, configuration, NoTransition.INSTANCE);
   }
 

src/test/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapperTest.java

@@ -43,7 +43,7 @@ public class AspectAwareAttributeMapperTest extends BuildViewTestCase {
   @Before
   public final void createMapper() throws Exception {
     ConfiguredTargetAndData ctad =
-        scratchConfiguredTargetAndTarget(
+        scratchConfiguredTargetAndData(
             "foo",
             "myrule",
             "cc_binary(",

src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java

@@ -151,16 +151,15 @@ public void testSourceArtifact() throws Exception {
   public void testGeneratedArtifact() throws Exception {
     setupDummyRule();
     update("//pkg:a.out");
-    OutputFileConfiguredTarget output =
-        (OutputFileConfiguredTarget) getConfiguredTarget("//pkg:a.out");
+    ConfiguredTargetAndData ctad = getConfiguredTargetAndData("//pkg:a.out");
+    OutputFileConfiguredTarget output = (OutputFileConfiguredTarget) ctad.getConfiguredTarget();
     Artifact outputArtifact = output.getArtifact();
     assertThat(outputArtifact.getRoot())
         .isEqualTo(
-            output
-                .getConfiguration()
+            ctad.getConfiguration()
                 .getBinDirectory(output.getLabel().getPackageIdentifier().getRepository()));
     assertThat(outputArtifact.getExecPath())
-        .isEqualTo(output.getConfiguration().getBinFragment().getRelative("pkg/a.out"));
+        .isEqualTo(ctad.getConfiguration().getBinFragment().getRelative("pkg/a.out"));
     assertThat(outputArtifact.getRootRelativePath()).isEqualTo(PathFragment.create("pkg/a.out"));
 
     Action action = getGeneratingAction(outputArtifact);

src/test/java/com/google/devtools/build/lib/analysis/CompilationHelperTest.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import java.io.IOException;
 import java.util.List;
 import org.junit.Before;
@@ -59,9 +60,9 @@ private List<Artifact> getAggregatingMiddleman(
         configuration);
   }
 
-  private List<Artifact> getAggregatingMiddleman(ConfiguredTarget rule, boolean withSolib)
+  private List<Artifact> getAggregatingMiddleman(ConfiguredTargetAndData rule, boolean withSolib)
       throws Exception {
-    return getAggregatingMiddleman(rule, rule.getConfiguration(), withSolib);
+    return getAggregatingMiddleman(rule.getConfiguredTarget(), rule.getConfiguration(), withSolib);
   }
 
   /**
@@ -71,8 +72,9 @@ private List<Artifact> getAggregatingMiddleman(ConfiguredTarget rule, boolean wi
    */
   @Test
   public void testDuplicateCallsReturnSameObject() throws Exception {
-    ConfiguredTarget rule =
-        scratchConfiguredTarget("package", "a", "cc_binary(name = 'a'," + "    srcs = ['a.cc'])");
+    ConfiguredTargetAndData rule =
+        scratchConfiguredTargetAndData(
+            "package", "a", "cc_binary(name = 'a'," + "    srcs = ['a.cc'])");
     List<Artifact> middleman1 = getAggregatingMiddleman(rule, false);
     assertThat(middleman1).hasSize(1);
     List<Artifact> middleman2 = getAggregatingMiddleman(rule, false);
@@ -88,8 +90,9 @@ public void testDuplicateCallsReturnSameObject() throws Exception {
    */
   @Test
   public void testMiddlemanAndSolibMiddlemanAreDistinct() throws Exception {
-    ConfiguredTarget rule = scratchConfiguredTarget("package", "liba.so",
-        "cc_binary(name = 'liba.so', srcs = ['a.cc'], linkshared = 1)");
+    ConfiguredTargetAndData rule =
+        scratchConfiguredTargetAndData(
+            "package", "liba.so", "cc_binary(name = 'liba.so', srcs = ['a.cc'], linkshared = 1)");
 
     List<Artifact> middleman = getAggregatingMiddleman(rule, false);
     assertThat(middleman).hasSize(1);
@@ -109,10 +112,10 @@ public void testPythonCcConfigurations() throws Exception {
 
     // Equivalent cc / Python configurations:
 
-    ConfiguredTarget ccRuleA = getConfiguredTarget("//foo:liba.so");
+    ConfiguredTargetAndData ccRuleA = getConfiguredTargetAndData("//foo:liba.so");
     List<Artifact> middleman1 = getAggregatingMiddleman(ccRuleA, true);
     try {
-      ConfiguredTarget ccRuleB = getConfiguredTarget("//foo:libb.so");
+      ConfiguredTargetAndData ccRuleB = getConfiguredTargetAndData("//foo:libb.so");
       getAggregatingMiddleman(ccRuleB, true);
       analysisEnvironment.registerWith(getMutableActionGraph());
       fail("Expected ActionConflictException due to same middleman artifact with different files");
@@ -124,9 +127,9 @@ public void testPythonCcConfigurations() throws Exception {
     // This should succeed because the py_binary's middleman is under the Python configuration's
     // internal directory, while the cc_binary's middleman is under the cc config's directory,
     // and both configurations are the same.
-    ConfiguredTarget pyRuleB = getDirectPrerequisite(
-        getConfiguredTarget("//foo:c"), "//foo:libb.so");
-
+    ConfiguredTargetAndData pyRuleB =
+        getConfiguredTargetAndDataDirectPrerequisite(
+            getConfiguredTargetAndData("//foo:c"), "//foo:libb.so");
 
     List<Artifact> middleman2 = getAggregatingMiddleman(pyRuleB, true);
     assertThat(Iterables.getOnlyElement(middleman2).getExecPathString())
@@ -144,10 +147,10 @@ public void testJavaCcConfigurations() throws Exception {
 
     // Equivalent cc / Java configurations:
 
-    ConfiguredTarget ccRuleA = getConfiguredTarget("//foo:liba.so");
+    ConfiguredTargetAndData ccRuleA = getConfiguredTargetAndData("//foo:liba.so");
     List<Artifact> middleman1 = getAggregatingMiddleman(ccRuleA, true);
     try {
-      ConfiguredTarget ccRuleB = getConfiguredTarget("//foo:libb.so");
+      ConfiguredTargetAndData ccRuleB = getConfiguredTargetAndData("//foo:libb.so");
       getAggregatingMiddleman(ccRuleB, true);
       analysisEnvironment.registerWith(getMutableActionGraph());
       fail("Expected ActionConflictException due to same middleman artifact with different files");
@@ -158,8 +161,9 @@ public void testJavaCcConfigurations() throws Exception {
 
     // This should succeed because the java_binary's middleman is under the Java configuration's
     // internal directory, while the cc_binary's middleman is under the cc config's directory.
-    ConfiguredTarget javaRuleB = getDirectPrerequisite(
-        getConfiguredTarget("//foo:d"), "//foo:libb.so");
+    ConfiguredTargetAndData javaRuleB =
+        getConfiguredTargetAndDataDirectPrerequisite(
+            getConfiguredTargetAndData("//foo:d"), "//foo:libb.so");
     List<Artifact> middleman2 = getAggregatingMiddleman(javaRuleB, false);
     assertThat(
             Iterables.getOnlyElement(middleman1)

src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java

@@ -448,13 +448,13 @@ public void computedDefaults() throws Exception {
 
     // Configuration a:
     useConfiguration("--test_arg=a");
-    ConfiguredTargetAndData binary = getConfiguredTargetAndTarget("//test:the_rule");
+    ConfiguredTargetAndData binary = getConfiguredTargetAndData("//test:the_rule");
     AttributeMap attributes = getMapperFromConfiguredTargetAndTarget(binary);
     assertThat(attributes.get("$computed_attr", Type.STRING)).isEqualTo("a2");
 
     // configuration b:
     useConfiguration("--test_arg=b");
-    binary = getConfiguredTargetAndTarget("//test:the_rule");
+    binary = getConfiguredTargetAndData("//test:the_rule");
     attributes = getMapperFromConfiguredTargetAndTarget(binary);
     assertThat(attributes.get("$computed_attr", Type.STRING)).isEqualTo("b2");
   }
@@ -1094,7 +1094,7 @@ public void selectableDefaultValueWithTypeDefault() throws Exception {
         "    }))");
 
     useConfiguration("--test_arg=a");
-    ConfiguredTargetAndData ctad = getConfiguredTargetAndTarget("//srctest:gen");
+    ConfiguredTargetAndData ctad = getConfiguredTargetAndData("//srctest:gen");
     AttributeMap attributes = getMapperFromConfiguredTargetAndTarget(ctad);
     assertThat(attributes.get("srcs", BuildType.LABEL_LIST)).isEmpty();
   }
@@ -1113,7 +1113,7 @@ public void selectableDefaultValueWithRuleDefault() throws Exception {
         "    boolean_attr = 1)");
 
     useConfiguration("--test_arg=a");
-    ConfiguredTargetAndData ctad = getConfiguredTargetAndTarget("//foo:rule");
+    ConfiguredTargetAndData ctad = getConfiguredTargetAndData("//foo:rule");
     AttributeMap attributes = getMapperFromConfiguredTargetAndTarget(ctad);
     assertThat(attributes.get("dep", BuildType.LABEL)).isEqualTo(
         Label.parseAbsolute("//foo:default"));

src/test/java/com/google/devtools/build/lib/analysis/ConfiguredAttributeMapperTest.java

@@ -46,7 +46,7 @@ public class ConfiguredAttributeMapperTest extends BuildViewTestCase {
    * Returns a ConfiguredAttributeMapper bound to the given rule with the target configuration.
    */
   private ConfiguredAttributeMapper getMapper(String label) throws Exception {
-    ConfiguredTargetAndData ctad = getConfiguredTargetAndTarget(label);
+    ConfiguredTargetAndData ctad = getConfiguredTargetAndData(label);
     return getMapperFromConfiguredTargetAndTarget(ctad);
   }
 

src/test/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTargetTest.java

@@ -18,8 +18,8 @@
 import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestBase;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
-
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -101,9 +101,10 @@ public void hostConfigSwitch() throws Exception {
     useConfiguration();
     update("//foo:gen3");
 
-    OutputFileConfiguredTarget hostSrc3 = (OutputFileConfiguredTarget)
-        getConfiguredTarget("//foo:host_src3.cc", getHostConfiguration());
-    TransitiveInfoCollection hostGeneratedFileConsumer3 = hostSrc3.getGeneratingRule();
+    ConfiguredTargetAndData hostSrc3 =
+        getConfiguredTargetAndData("//foo:host_src3.cc", getHostConfiguration());
+    TransitiveInfoCollection hostGeneratedFileConsumer3 =
+        ((OutputFileConfiguredTarget) hostSrc3.getConfiguredTarget()).getGeneratingRule();
     assertThat(hostSrc3.getConfiguration())
         .isEqualTo(hostGeneratedFileConsumer3.getConfiguration());
     // TODO(gregce): enable below for Bazel tests, which for some reason realize the same instance

src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java

@@ -404,7 +404,7 @@ protected ConfiguredTargetAndData getConfiguredTargetAndTarget(
     } catch (LabelSyntaxException e) {
       throw new AssertionError(e);
     }
-    return skyframeExecutor.getConfiguredTargetAndTargetForTesting(reporter, parsedLabel, config);
+    return skyframeExecutor.getConfiguredTargetAndDataForTesting(reporter, parsedLabel, config);
   }
 
   protected Target getTarget(String label) throws InterruptedException {
@@ -416,11 +416,23 @@ protected Target getTarget(String label) throws InterruptedException {
     }
   }
 
-  protected ConfiguredTarget getConfiguredTarget(String label, BuildConfiguration configuration) {
+  protected final ConfiguredTargetAndData getConfiguredTargetAndData(
+      String label, BuildConfiguration configuration) {
     ensureUpdateWasCalled();
     return getConfiguredTargetForSkyframe(label, configuration);
   }
 
+  protected final ConfiguredTargetAndData getConfiguredTargetAndData(String label)
+      throws InterruptedException {
+    return getConfiguredTargetAndData(label, getTargetConfiguration());
+  }
+
+  protected final ConfiguredTarget getConfiguredTarget(
+      String label, BuildConfiguration configuration) {
+    ConfiguredTargetAndData result = getConfiguredTargetAndData(label, configuration);
+    return result == null ? null : result.getConfiguredTarget();
+  }
+
   /**
    * Returns the corresponding configured target, if it exists. Note that this will only return
    * anything useful after a call to update() with the same label.
@@ -429,15 +441,16 @@ protected ConfiguredTarget getConfiguredTarget(String label) throws InterruptedE
     return getConfiguredTarget(label, getTargetConfiguration());
   }
 
-  private ConfiguredTarget getConfiguredTargetForSkyframe(String label,
-      BuildConfiguration configuration) {
+  private ConfiguredTargetAndData getConfiguredTargetForSkyframe(
+      String label, BuildConfiguration configuration) {
     Label parsedLabel;
     try {
       parsedLabel = Label.parseAbsolute(label);
     } catch (LabelSyntaxException e) {
       throw new AssertionError(e);
     }
-    return skyframeExecutor.getConfiguredTargetForTesting(reporter, parsedLabel, configuration);
+    return skyframeExecutor.getConfiguredTargetAndDataForTesting(
+        reporter, parsedLabel, configuration);
   }
 
   /**