handling for debugging python test targets with transitions

aspect/python_info.bzl

@@ -1,6 +1,6 @@
 # TEMPLATE-INCLUDE-BEGIN
-###if( $isPythonEnabled == "true" && $bazel8OrAbove == "true" )
-##load("@rules_python//python:defs.bzl", "PyInfo")
+###if( $isPythonEnabled == "true" )
+##load("@rules_python//python:defs.bzl", RulesPyInfo = "PyInfo")
 ###end
 # TEMPLATE-INCLUDE-END
 
@@ -10,11 +10,15 @@ def py_info_in_target(target):
 # TEMPLATE-IGNORE-END
 
 # TEMPLATE-INCLUDE-BEGIN
-##  #if( $isPythonEnabled == "true" )
-##  return PyInfo in target
-##  #else
-##  return None
-##  #end
+###if( $isPythonEnabled == "true" )
+##  if RulesPyInfo in target:
+##    return True
+###end
+###if( $bazel8OrAbove != "true")
+##  if PyInfo in target:
+##    return True
+###end
+##  return False
 # TEMPLATE-INCLUDE-END
 
 def get_py_info(target):
@@ -26,12 +30,14 @@ def get_py_info(target):
 # TEMPLATE-IGNORE-END
 
 # TEMPLATE-INCLUDE-BEGIN
-##  #if( $isPythonEnabled == "true" )
+###if( $isPythonEnabled == "true" )
+##  if RulesPyInfo in target:
+##    return target[RulesPyInfo]
+###end
+###if( $bazel8OrAbove != "true")
 ##  if PyInfo in target:
-##      return target[PyInfo]
-##  else:
-##      return None
-##  #else
+##    return target[PyInfo]
+###end
 ##  return None
-##  #end
-# TEMPLATE-INCLUDE-END
+# TEMPLATE-INCLUDE-END
+

base/src/com/google/idea/blaze/base/run/testmap/ProjectSourceToTargetFinder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 The Bazel Authors. All rights reserved.
+ * Copyright 2016-2024 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.
@@ -37,10 +37,12 @@
 import com.intellij.openapi.project.Project;
 import java.io.File;
 import java.util.Collection;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Future;
+import java.util.stream.Collectors;
 
 /**
  * Used to locate tests from source files for things like right-clicks.
@@ -58,7 +60,7 @@ public Future<Collection<TargetInfo>> targetsForSourceFiles(
       if (projectData == null) {
         return Futures.immediateFuture(ImmutableList.of());
       }
-      ImmutableSet<TargetInfo> targets =
+      List<TargetInfo> targets =
           sourceFiles.stream()
               .map(file -> projectData.getWorkspacePathResolver().getWorkspacePath(file))
               .filter(Objects::nonNull)
@@ -76,7 +78,9 @@ public Future<Collection<TargetInfo>> targetsForSourceFiles(
                     return kind.getRuleType().equals(ruleType.get());
                   })
               .map(TargetInfo::fromBuildTarget)
-              .collect(toImmutableSet());
+              .distinct()
+              .sorted(new TargetInfoComparator())
+              .collect(Collectors.toList());
       return Futures.immediateFuture(targets);
     }
     FilteredTargetMap targetMap =
@@ -85,11 +89,13 @@ public Future<Collection<TargetInfo>> targetsForSourceFiles(
     if (targetMap == null) {
       return Futures.immediateFuture(ImmutableList.of());
     }
-    ImmutableSet<TargetInfo> targets =
+    List<TargetInfo> targets =
         targetMap.targetsForSourceFiles(sourceFiles).stream()
             .map(TargetIdeInfo::toTargetInfo)
             .filter(target -> !ruleType.isPresent() || target.getRuleType().equals(ruleType.get()))
-            .collect(toImmutableSet());
+            .distinct()
+            .sorted(new TargetInfoComparator())
+            .collect(Collectors.toList());
     return Futures.immediateFuture(targets);
   }
 

base/src/com/google/idea/blaze/base/run/testmap/TargetInfoComparator.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright 2024 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.idea.blaze.base.run.testmap;
+
+import com.google.idea.blaze.base.dependencies.TargetInfo;
+import com.google.idea.blaze.base.model.primitives.Kind;
+import java.util.Comparator;
+
+public class TargetInfoComparator implements Comparator<TargetInfo> {
+
+  /**
+   * Sorts the {@link TargetInfo} objects such that there is a preference to those without the
+   * underscore on the name and for those that do actually resolve to a {@link Kind}.
+   */
+
+  @Override
+  public int compare(TargetInfo o1, TargetInfo o2) {
+    Kind kind1 = o1.getKind();
+    Kind kind2 = o2.getKind();
+
+    if ((null == kind1) != (null == kind2)) {
+      return (null == kind1) ? 1 : -1;
+    }
+
+    String targetNameStr1 = o1.getLabel().targetName().toString();
+    String targetNameStr2 = o2.getLabel().targetName().toString();
+
+    boolean targetNameLeadingUnderscore1 = targetNameStr1.startsWith("_");
+    boolean targetNameLeadingUnderscore2 = targetNameStr2.startsWith("_");
+
+    if (targetNameLeadingUnderscore1 != targetNameLeadingUnderscore2) {
+      return targetNameLeadingUnderscore1 ? 1 : -1;
+    }
+
+    return targetNameStr1.compareTo(targetNameStr2);
+  }
+
+}

base/tests/unittests/com/google/idea/blaze/base/run/testmap/TargetInfoComparatorTest.java

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2024 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.idea.blaze.base.run.testmap;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.idea.blaze.base.BlazeTestCase;
+import com.google.idea.blaze.base.dependencies.TargetInfo;
+import com.google.idea.blaze.base.model.primitives.GenericBlazeRules;
+import com.google.idea.blaze.base.model.primitives.Kind;
+import com.google.idea.blaze.base.model.primitives.Label;
+import com.intellij.openapi.extensions.impl.ExtensionPointImpl;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class TargetInfoComparatorTest extends BlazeTestCase {
+
+  private TargetInfoComparator comparator = new TargetInfoComparator();
+
+  @Override
+  protected void initTest(Container applicationServices, Container projectServices) {
+    super.initTest(applicationServices, projectServices);
+    ExtensionPointImpl<Kind.Provider> kindProvider =
+        registerExtensionPoint(Kind.Provider.EP_NAME, Kind.Provider.class);
+    kindProvider.registerExtension(new GenericBlazeRules());
+    applicationServices.register(Kind.ApplicationState.class, new Kind.ApplicationState());
+  }
+
+  /**
+   * <p>In this case the `zzz` kindString will not resolve to a known instance of
+   * {@link com.google.idea.blaze.base.model.primitives.Kind} and so it will come last.</p>
+   */
+
+  @Test
+  public void testNoKind() {
+    List<TargetInfo> targetInfos = List.of(
+        TargetInfo.builder(Label.create("//project:aaa"), "zzz").build(),
+        TargetInfo.builder(Label.create("//project:bbb"), "sh_test").build()
+    );
+
+    List<TargetInfo> sortedTargetInfos = targetInfos
+        .stream()
+        .sorted(comparator)
+        .collect(Collectors.toUnmodifiableList());
+
+    // expecting that the first one will be the one that has the Kind.
+    assertThat(sortedTargetInfos.get(0).getLabel()).isEqualTo(Label.create("//project:bbb"));
+    assertThat(sortedTargetInfos.get(1).getLabel()).isEqualTo(Label.create("//project:aaa"));
+  }
+
+  /**
+   * <p>In this case the `_transition_` will be stopped on the `_transition_py_test` and so
+   * both {@link TargetInfo}s will be having the
+   * {@link com.google.idea.blaze.base.model.primitives.Kind} of `py_test`. This means that the
+   * sorting will make the `_aaa` last as it starts with an underscore.</p>
+   */
+  @Test
+  public void testUnderscoreOnLabelTargetName() {
+    List<TargetInfo> targetInfos = List.of(
+        TargetInfo.builder(Label.create("//project:_aaa"), "sh_test").build(),
+        TargetInfo.builder(Label.create("//project:bbb"), "_transition_sh_test").build()
+    );
+
+    List<TargetInfo> sortedTargetInfos = targetInfos
+        .stream()
+        .sorted(comparator)
+        .collect(Collectors.toUnmodifiableList());
+
+    // expecting that the first one will be the one that has the Kind.
+    assertThat(sortedTargetInfos.get(0).getLabel()).isEqualTo(Label.create("//project:bbb"));
+    assertThat(sortedTargetInfos.get(1).getLabel()).isEqualTo(Label.create("//project:_aaa"));
+  }
+
+  /**
+   * <p>As there is no missing {@link com.google.idea.blaze.base.model.primitives.Kind}
+   * nor an underscore-prefixed </p>
+   */
+  @Test
+  public void testByLabelName() {
+    List<TargetInfo> targetInfos = List.of(
+        TargetInfo.builder(Label.create("//project:ddd"), "sh_test").build(),
+        TargetInfo.builder(Label.create("//project:aaa"), "sh_test").build()
+    );
+
+    List<TargetInfo> sortedTargetInfos = targetInfos
+        .stream()
+        .sorted(comparator)
+        .collect(Collectors.toUnmodifiableList());
+
+    // expecting that the first one will be the one that has the Kind.
+    assertThat(sortedTargetInfos.get(0).getLabel()).isEqualTo(Label.create("//project:aaa"));
+    assertThat(sortedTargetInfos.get(1).getLabel()).isEqualTo(Label.create("//project:ddd"));
+  }
+
+}

querysync/java/com/google/idea/blaze/qsync/BlazeQueryParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 The Bazel Authors. All rights reserved.
+ * Copyright 2022-2024 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.
@@ -16,8 +16,6 @@
 package com.google.idea.blaze.qsync;
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.idea.blaze.common.Label.toLabelList;
-import static java.util.Arrays.stream;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ImmutableMap;
@@ -29,11 +27,9 @@
 import com.google.idea.blaze.common.PrintOutput;
 import com.google.idea.blaze.common.RuleKinds;
 import com.google.idea.blaze.qsync.project.BuildGraphData;
-import com.google.idea.blaze.qsync.project.BuildGraphData.Location;
 import com.google.idea.blaze.qsync.project.ProjectTarget;
 import com.google.idea.blaze.qsync.project.ProjectTarget.SourceType;
 import com.google.idea.blaze.qsync.project.QuerySyncLanguage;
-import com.google.idea.blaze.qsync.query.Query;
 import com.google.idea.blaze.qsync.query.QueryData;
 import com.google.idea.blaze.qsync.query.QuerySummary;
 import java.util.HashSet;
@@ -47,8 +43,15 @@
  */
 public class BlazeQueryParser {
 
-  // Rules that will need to be built, whether or not the target is included in the
-  // project.
+  /**
+   * When a rule has a transition applied to it then it will present with this
+   * prefix on it.
+   */
+
+  private final static String RULE_NAME_PREFIX_TRANSITION = "_transition_";
+
+  // Rules that will need to be built, whether the target is included in the
+  // project or not.
   public static final ImmutableSet<String> ALWAYS_BUILD_RULE_KINDS =
       ImmutableSet.of(
           "java_proto_library",
@@ -110,6 +113,17 @@ private static void register(ImmutableMap.Builder<String, RuleVisitor> builder,
                                  RuleVisitor visitor) {
       for (String kind : kinds) {
         builder.put(kind, visitor);
+
+        // When the `bazel query` executes over Rules which have transitions applied to them, the
+        // returned list of data contains rules which have Rule names with `_` prefixed but with
+        // the original Kind and _also_ original Rule names with the Kind prefixed with
+        // `_transition_`. An example is when a specific pinned Python version is used, a
+        // transition is employed to enforce the vesion. In this case we see a rule `_my_rule` with
+        // kind `py_test` and a rule `my_rule` with kind `_transition_py_test`. Without
+        // accommodating for this, the `_transition_py_test` one would be ignored and so the
+        // downstream logic here would be working with the wrong Rule name.
+
+        builder.put(RULE_NAME_PREFIX_TRANSITION + kind, visitor);
       }
     }
   }

querysync/javatests/com/google/idea/blaze/qsync/query/QuerySpecTest.java

@@ -19,6 +19,8 @@
 
 import com.google.idea.blaze.qsync.BlazeQueryParser;
 import java.nio.file.Path;
+import java.util.stream.Stream;
+import java.util.stream.Collectors;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -86,6 +88,22 @@ public void testGetQueryExpression_includes_and_excludes() {
 
   @Test
   public void testGetQueryExpression_experimental_includes_and_excludes() {
+
+    // There are a number of Kinds that are expected to be queried for. It is also
+    // necessary to include the same Kinds with the prefix `_transition_` as well because it
+    // could be that any given rule is subject to a transition.
+
+    String kindsExpression = Stream.of(
+        "android_library", "android_binary", "android_local_test",
+        "android_instrumentation_test", "kt_android_library_helper", "java_library", "java_binary",
+        "kt_jvm_library", "kt_jvm_binary", "kt_jvm_library_helper", "kt_native_library", "java_test",
+        "java_proto_library", "java_lite_proto_library", "java_mutable_proto_library",
+        "_java_grpc_library", "_kotlin_library", "_java_lite_grpc_library", "_iml_module_",
+        "cc_library", "cc_binary", "cc_shared_library", "cc_test", "proto_library", "py_library",
+        "py_binary", "py_test")
+        .flatMap(k -> Stream.of(k, "_transition_" + k))
+        .collect(Collectors.joining("|"));
+
     QuerySpec qs =
       QuerySpec.builder(QuerySpec.QueryStrategy.FILTERING_TO_KNOWN_AND_USED_TARGETS)
         .workspaceRoot(Path.of("/workspace/"))
@@ -98,7 +116,7 @@ public void testGetQueryExpression_experimental_includes_and_excludes() {
     assertThat(qs.getQueryExpression())
       .hasValue(
         "let base = //some/included/path/...:* + //another/included/path/...:* - //some/included/path/excluded/...:* - //another/included/path/excluded/...:*\n" +
-        " in let known = kind(\"source file|android_library|android_binary|android_local_test|android_instrumentation_test|kt_android_library_helper|java_library|java_binary|kt_jvm_library|kt_jvm_binary|kt_jvm_library_helper|kt_native_library|java_test|java_proto_library|java_lite_proto_library|java_mutable_proto_library|_java_grpc_library|_kotlin_library|_java_lite_grpc_library|_iml_module_|cc_library|cc_binary|cc_shared_library|cc_test|proto_library|py_library|py_binary|py_test\", $base) \n" +
+        " in let known = kind(\"source file|" + kindsExpression + "\", $base) \n" +
         " in let unknown = $base except $known \n" +
         " in $known union ($base intersect allpaths($known, $unknown)) \n");
   }