Address comments

bazelbuild · Jan 25, 2024 · 5136a16 · 5136a16
1 parent 95d4364
commit 5136a16
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 12 deletions.
diff --git a/java/src/com/google/idea/blaze/java/run/JUnitTestHeuristic.java b/java/src/com/google/idea/blaze/java/run/JUnitTestHeuristic.java
@@ -50,7 +50,8 @@ public static Optional<JUnitVersion> jUnitVersion(PsiClass psiClass) {
     } else if (JUnitUtil.isJUnit3TestClass(psiClass)) {
       return Optional.of(JUnitVersion.JUNIT_3);
     } else {
-      return Optional.empty();
+      // Fall back to Junit3
+      return Optional.of(JUnitVersion.JUNIT_3);
     }
   }
 
@@ -82,7 +83,7 @@ public boolean matchesSource(
       File sourceFile,
       @Nullable TestSize testSize) {
     Optional<JUnitVersion> sourceVersion = junitVersion(sourcePsiFile);
-    if (sourceVersion == null || sourceVersion.isEmpty()) {
+    if (sourceVersion.isEmpty()) {
       return false;
     }
     String targetName = target.label.targetName().toString().toLowerCase();
@@ -97,15 +98,13 @@ public boolean matchesSource(
     return false;
   }
 
-  @Nullable
   private Optional<JUnitVersion> junitVersion(@Nullable PsiFile psiFile) {
     if (!(psiFile instanceof PsiClassOwner)) {
-      return null;
+      return Optional.empty();
     }
     return ReadAction.compute(() -> junitVersion((PsiClassOwner) psiFile));
   }
 
-  @Nullable
   private Optional<JUnitVersion> junitVersion(PsiClassOwner classOwner) {
     for (PsiClass psiClass : classOwner.getClasses()) {
       Optional<JUnitVersion> version = jUnitVersion(psiClass);

diff --git a/java/src/com/google/idea/blaze/java/run/producers/BlazeJUnitTestFilterFlags.java b/java/src/com/google/idea/blaze/java/run/producers/BlazeJUnitTestFilterFlags.java
@@ -122,7 +122,7 @@ public static String testFilterForClassesAndMethods(
     // java_test target they're run from.
     Optional<JUnitVersion> version = JUnitTestHeuristic.jUnitVersion(methodsPerClass.keySet());
     if (version.isEmpty()) {
-      return null;
+      return testFilterForClassesAndMethods(methodsPerClass, JUnitVersion.JUNIT_3);
     }
     return testFilterForClassesAndMethods(methodsPerClass, version.get());
   }

diff --git a/.../com/google/idea/blaze/java/run/producers/MultipleJavaClassesTestContextProviderTest.java b/.../com/google/idea/blaze/java/run/producers/MultipleJavaClassesTestContextProviderTest.java
@@ -41,6 +41,7 @@
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiFile;
 import com.intellij.psi.PsiJavaFile;
+
 import java.util.Arrays;
 import java.util.List;
 import org.junit.After;
@@ -88,6 +89,11 @@ public final void removeSourceFolder() {
               final ModifiableRootModel model =
                   ModuleRootManager.getInstance(testFixture.getModule()).getModifiableModel();
               ContentEntry contentEntry = model.getContentEntries()[0];
+              // There is a global content entry, `//src`, which is necessary to infer targets from test classes.
+              // This is set outside of these test (somewhere in the superclass) once per run, not once per test.
+              // If we just use contentEntry.clearSourceDirs() here, it won't be re-created, and tests that assume
+              // it exists will fail.
+              // Therefore, we only delete the content entries that this test has created.
               Arrays.stream(contentEntry.getSourceFolders()).forEach((folder) -> {
                 if (folder.getFile().equals(pkgRoot)) {
                   contentEntry.removeSourceFolder(folder);

diff --git a/...sts/unittests/com/google/idea/blaze/java/run/producers/BlazeJUnitTestFilterFlagsTest.java b/...sts/unittests/com/google/idea/blaze/java/run/producers/BlazeJUnitTestFilterFlagsTest.java
@@ -64,19 +64,25 @@ public void testSingleJUnit3ClassFilter() {
   }
 
   @Test
-  public void testParameterizedIgnoredForSingleClass() {
+  public void testParameterizedIgnoredForSingleClassJUnit4() {
     assertThat(
             BlazeJUnitTestFilterFlags.testFilterForClassAndMethods(
-                "com.google.idea.ClassName", JUnitVersion.JUNIT_4, ImmutableList.of(), null))
-        .isEqualTo("com.google.idea.ClassName#");
+                    "com.google.idea.ClassName",
+                    JUnitVersion.JUNIT_4,
+                    ImmutableList.of(),
+                    ParameterizedTestInfo.create("ignored", "-ignored")))
+            .isEqualTo("com.google.idea.ClassName#");
   }
 
   @Test
   public void testParameterizedIgnoredForSingleClassJUnit5() {
     assertThat(
-        BlazeJUnitTestFilterFlags.testFilterForClassAndMethods(
-            "com.google.idea.ClassName", JUnitVersion.JUNIT_5, ImmutableList.of(), null))
-        .isEqualTo("com.google.idea.ClassName#");
+            BlazeJUnitTestFilterFlags.testFilterForClassAndMethods(
+                    "com.google.idea.ClassName",
+                    JUnitVersion.JUNIT_5,
+                    ImmutableList.of(),
+                    ParameterizedTestInfo.create("ignored", "-ignored")))
+            .isEqualTo("com.google.idea.ClassName#");
   }
 
   @Test