commit ef758fa57c31ed7c04937c3b4a263a0e25315bc2
author Googler <yuyueny@google.com> Mon Dec 18 09:13:50 2023 -0800
committer Copybara-Service <copybara-worker@google.com> Mon Dec 18 09:15:22 2023 -0800
tree 04a77d22d9a7be158590c1b709c6acf05cbc2c0d
parent aa1e0b431f1eac8acb359e5e13b1a209f8c2e246

Automated rollback of commit 07eb2d77f94f9e11e64082dab9581ad7acf44dc7.

*** Reason for rollback ***

Re-fix the `subpackages()` bug when considering a corner case

In the initial change, we have introduced another bug:

Consider computing `subpackages(["sub/**"])` while `sub/BUILD` exists, we should return "sub", which is the behavior of file/directory glob matching. However, the initial fix returns EMPTY in this case.

This change also adds more test coverage for calling `subpackages(["sub/**"])` when `sub` is a subpackage.

PiperOrigin-RevId: 591917000
Change-Id: Iab41cc59b0da7f7794a90cd10aaac8900ee0d79b

src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index db5b460..0469085 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -338,11 +338,54 @@
   }
 
   @Test
-  public void testSubpackages() throws Exception {
+  public void testSubpackages_noWildcard() throws Exception {
+    assertThat(cache.globUnsorted(list("sub/sub.js"), list(), Globber.Operation.SUBPACKAGES, true))
+        .isEmpty();
+  }
+
+  @Test
+  public void testSubpackages_simpleDoubleStar() throws Exception {
     assertThat(cache.globUnsorted(list("**"), list(), Globber.Operation.SUBPACKAGES, true))
         .containsExactly("sub");
   }
 
+  @Test
+  public void testSubpackages_onlySub() throws Exception {
+    assertThat(cache.globUnsorted(list("sub"), list(), Globber.Operation.SUBPACKAGES, true))
+        .containsExactly("sub");
+  }
+
+  @Test
+  public void testSubpackages_singleStarsAfterSub() throws Exception {
+    assertThat(cache.globUnsorted(list("sub/*"), list(), Globber.Operation.SUBPACKAGES, true))
+        .isEmpty();
+  }
+
+  @Test
+  public void testSubpackages_doubleStarsAfterSub() throws Exception {
+    assertThat(cache.globUnsorted(list("sub/**"), list(), Globber.Operation.SUBPACKAGES, true))
+        .containsExactly("sub");
+  }
+
+  @Test
+  public void testSubpackages_twoDoubleStarsAfterSub() throws Exception {
+    // Both `**`s are considered to match no path fragments.
+    assertThat(cache.globUnsorted(list("sub/**/**"), list(), Globber.Operation.SUBPACKAGES, true))
+        .containsExactly("sub");
+  }
+
+  @Test
+  public void testSubpackages_doubleStarsAndOtherPathAfterSub() throws Exception {
+    assertThat(cache.globUnsorted(list("sub/**/foo"), list(), Globber.Operation.SUBPACKAGES, true))
+        .isEmpty();
+  }
+
+  @Test
+  public void testSubpackages_doubleStarWithTrailingPattern() throws Exception {
+    assertThat(cache.globUnsorted(list("**/bar"), list(), Globber.Operation.SUBPACKAGES, true))
+        .isEmpty();
+  }
+
   private void assertEmpty(Collection<?> glob) {
     assertThat(glob).isEmpty();
   }