Fix flaky Spec11PipelineTest (#1133)

core/src/test/java/google/registry/beam/TestPipelineExtension.java

@@ -20,6 +20,7 @@
 package google.registry.beam;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.truth.Truth.assertWithMessage;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
@@ -27,6 +28,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -295,6 +297,16 @@ public class TestPipelineExtension extends Pipeline
 
       enableAbandonedNodeEnforcement(true);
     }
+
+    // Clear this property so that when default Guava ThreadFactory is created
+    // it will not think that it is in App Engine and return an unusable
+    // ThreadFactory.
+    System.clearProperty("com.google.appengine.runtime.environment");
+    assertWithMessage(
+            "Beam pipelines don't run in an App Engine environment, and thus"
+                + " the tests shouldn't be mocking one either.")
+        .that(isAppEngine())
+        .isFalse();
   }
 
   @Override
@@ -500,6 +512,25 @@ public class TestPipelineExtension extends Pipeline
     }
   }
 
+  // Adapted from Guava's MoreExecutors (where it is a private method)
+  private static boolean isAppEngine() {
+    if (System.getProperty("com.google.appengine.runtime.environment") == null) {
+      return false;
+    } else {
+      try {
+        return Class.forName("com.google.apphosting.api.ApiProxy")
+                .getMethod("getCurrentEnvironment")
+                .invoke(null)
+            != null;
+      } catch (ClassNotFoundException
+          | InvocationTargetException
+          | IllegalAccessException
+          | NoSuchMethodException e) {
+        return false;
+      }
+    }
+  }
+
   private static class IsEmptyVisitor extends PipelineVisitor.Defaults {
     private boolean empty = true;
 

core/src/test/java/google/registry/beam/spec11/Spec11PipelineTest.java

@@ -16,7 +16,6 @@ package google.registry.beam.spec11;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth.assertWithMessage;
 import static google.registry.model.ImmutableObjectSubject.immutableObjectCorrespondence;
 import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
 
@@ -35,7 +34,6 @@ import google.registry.testing.DatastoreEntityExtension;
 import google.registry.testing.FakeClock;
 import google.registry.util.ResourceUtils;
 import java.io.File;
-import java.lang.reflect.InvocationTargetException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import org.apache.beam.sdk.coders.KvCoder;
@@ -120,11 +118,6 @@ class Spec11PipelineTest {
                     KvCoder.of(
                         SerializableCoder.of(Subdomain.class),
                         SerializableCoder.of(ThreatMatch.class))));
-    assertWithMessage(
-            "Beam pipelines don't run in an App Engine environment, and thus the tests shouldn't be"
-                + " mocking one either")
-        .that(isAppEngine())
-        .isFalse();
   }
 
   @Test
@@ -195,25 +188,6 @@ class Spec11PipelineTest {
         .containsExactlyElementsIn(expectedFileContents.subList(1, expectedFileContents.size()));
   }
 
-  // Adapted from Guava's MoreExecutors (where it is a private method)
-  private static boolean isAppEngine() {
-    if (System.getProperty("com.google.appengine.runtime.environment") == null) {
-      return false;
-    } else {
-      try {
-        return Class.forName("com.google.apphosting.api.ApiProxy")
-                .getMethod("getCurrentEnvironment")
-                .invoke(null)
-            != null;
-      } catch (ClassNotFoundException
-          | InvocationTargetException
-          | IllegalAccessException
-          | NoSuchMethodException e) {
-        return false;
-      }
-    }
-  }
-
   /** Returns the text contents of a file under the beamBucket/results directory. */
   private ImmutableList<String> resultFileContents() throws Exception {
     File resultFile =

core/src/test/java/google/registry/testing/DatastoreEntityExtension.java

@@ -26,9 +26,10 @@ import org.testcontainers.shaded.com.google.common.collect.ImmutableMap;
  * Allows instantiation of Datastore {@code Entity entities} without the heavyweight {@code
  * AppEngineRule}.
  *
- * <p>When used together with {@code JpaIntegrationWithCoverageExtension}, this extension must be
- * registered first. For consistency's sake, it is recommended that the field for this extension be
- * annotated with {@code @org.junit.jupiter.api.Order(value = 1)}. Please refer to {@link
+ * <p>When used together with {@code JpaIntegrationWithCoverageExtension} or @{@code
+ * TestPipelineExtension}, this extension must be registered first. For consistency's sake, it is
+ * recommended that the field for this extension be annotated with
+ * {@code @org.junit.jupiter.api.Order(value = 1)}. Please refer to {@link
  * google.registry.model.domain.DomainBaseSqlTest} for example, and to <a
  * href="https://junit.org/junit5/docs/current/user-guide/#extensions-registration-programmatic">
  * JUnit 5 User Guide</a> for details of extension ordering.