Actually run JUnit 5 tests (#545)

* Actually run JUnit 5 tests

In Gradle, JUnit 5 must be explicitly enabled with a call to
test.useJUnitPlatform().

The FilteringTest used in :core must also enable JUnit5 separately.

Fixed AppEngineRule to work with the few tests that have migrated
to JUnit5.

More work is needed with AppEngine before we can migrate tests
that actually use Cloud SQL.

For context, with @EnableRuleMigrationSupport, JUnit 5 runner calls
an external resource's before() and after() methods. TestRule.apply()
is not called, therefore any setup done their will be bypassed with
JUnit 5.

core/build.gradle

@@ -646,6 +646,10 @@ artifacts {
  */
 class FilteringTest extends Test {
 
+  FilteringTest() {
+    useJUnitPlatform();
+  }
+
   private void applyTestFilter() {
     if (project.testFilter) {
       testNameIncludePatterns = project.testFilter.split(',')

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

@@ -40,6 +40,7 @@ import google.registry.model.registrar.Registrar.State;
 import google.registry.model.registrar.RegistrarAddress;
 import google.registry.model.registrar.RegistrarContact;
 import google.registry.persistence.transaction.JpaTestRules;
+import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationWithCoverageRule;
 import google.registry.util.Clock;
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -256,7 +257,14 @@ public final class AppEngineRule extends ExternalResource {
         .build();
   }
 
-  /** Hack to make sure AppEngineRule is always wrapped in a TemporaryFolder rule. */
+  /**
+   * Hack to make sure AppEngineRule is always wrapped in a {@link JpaIntegrationWithCoverageRule}.
+   */
+  // Note: Even with @EnableRuleMigrationSupport, JUnit5 runner does not call this method.
+  // Note 2: Do not migrate any members of SqlIntegrationTestSuite to JUnit5 before
+  // calls to JpaIntegrationWithCoverageRule can be made elsewhere.
+  // TODO(weiminyu): make JpaIntegrationWithCoverageRule implement ExternaResource and invoke it in
+  // before() and after(), then drop this method.
   @Override
   public Statement apply(Statement base, Description description) {
     Statement statement = base;
@@ -267,12 +275,13 @@ public final class AppEngineRule extends ExternalResource {
       }
       statement = builder.buildIntegrationWithCoverageRule().apply(base, description);
     }
-    return temporaryFolder.apply(super.apply(statement, description), description);
+    return super.apply(statement, description);
   }
 
   @Override
   protected void before() throws IOException {
     setupLogging();
+    temporaryFolder.create();
     Set<LocalServiceTestConfig> configs = new HashSet<>();
     if (withUrlFetch) {
       configs.add(new LocalURLFetchServiceTestConfig());
@@ -346,10 +355,10 @@ public final class AppEngineRule extends ExternalResource {
     helper = null;
     // Test that Datastore didn't need any indexes we don't have listed in our index file.
     File indexFile = new File(temporaryFolder.getRoot(), "datastore-indexes-auto.xml");
+    try {
       if (!indexFile.exists()) {
         return;
       }
-    try {
       String indexFileContent = Files.asCharSource(indexFile, UTF_8).read();
       if (indexFileContent.trim().isEmpty()) {
         return;
@@ -361,6 +370,8 @@ public final class AppEngineRule extends ExternalResource {
       }
     } catch (IOException e) {
       throw new RuntimeException(e);
+    } finally {
+      temporaryFolder.delete();
     }
   }
 

java_common.gradle

@@ -72,6 +72,10 @@ dependencies {
     errorprone("com.google.errorprone:error_prone_core:2.3.3")
 }
 
+test {
+  useJUnitPlatform()
+}
+
 tasks.withType(JavaCompile).configureEach {
     // The -Werror flag causes Intellij to fail on deprecated api use.
     // Allow IDE user to turn off this flag by specifying a Gradle VM