diff --git a/javatests/google/registry/testing/AppEngineRule.java b/javatests/google/registry/testing/AppEngineRule.java index e03e19574..a5b554381 100644 --- a/javatests/google/registry/testing/AppEngineRule.java +++ b/javatests/google/registry/testing/AppEngineRule.java @@ -94,7 +94,7 @@ public final class AppEngineRule extends ExternalResource { private LocalServiceTestHelper helper; /** A rule-within-a-rule to provide a temporary folder for AppEngineRule's internal temp files. */ - private TemporaryFolder temporaryFolder = new TemporaryFolder(); + TemporaryFolder temporaryFolder = new TemporaryFolder(); private boolean withDatastore; private boolean withLocalModules; @@ -328,14 +328,22 @@ public final class AppEngineRule extends ExternalResource { helper.tearDown(); 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"); + if (!indexFile.exists()) { + return; + } try { - Set autoIndexes = getIndexXmlStrings(Files.asCharSource( - new File(temporaryFolder.getRoot(), "datastore-indexes-auto.xml"), UTF_8).read()); + String indexFileContent = Files.asCharSource(indexFile, UTF_8).read(); + if (indexFileContent.trim().isEmpty()) { + return; + } + Set autoIndexes = getIndexXmlStrings(indexFileContent); Set missingIndexes = Sets.difference(autoIndexes, MANUAL_INDEXES); if (!missingIndexes.isEmpty()) { assert_().fail("Missing indexes:\n%s", Joiner.on('\n').join(missingIndexes)); } - } catch (IOException e) { // This is fine; no indexes were written. + } catch (IOException e) { + throw new RuntimeException(e); } } @@ -352,12 +360,6 @@ public final class AppEngineRule extends ExternalResource { // To normalize the indexes, we are going to pass them through JSON and then rewrite the xml. JSONObject datastoreIndexes = new JSONObject(); JSONObject indexFileObject = toJSONObject(indexFile); - if (!indexFileObject.has("datastore-indexes")) { - // Log for flaky appearance. We suspect that this is not a problem but need to inspect - // the content of this file to make sure. - // TODO(weiminyu): inspect file content and decide if this should be ignored. - logger.atWarning().log("Unexpected datastore-indexes-auto.xml:\n%s", indexFileObject); - } Object indexes = indexFileObject.get("datastore-indexes"); if (indexes instanceof JSONObject) { datastoreIndexes = (JSONObject) indexes; @@ -366,7 +368,8 @@ public final class AppEngineRule extends ExternalResource { builder.add(getIndexXmlString(index)); } } catch (JSONException e) { - throw new RuntimeException(e); + throw new RuntimeException( + String.format("Error parsing datastore-indexes-auto.xml: [%s]", indexFile), e); } return builder.build(); } diff --git a/javatests/google/registry/testing/AppEngineRuleTest.java b/javatests/google/registry/testing/AppEngineRuleTest.java new file mode 100644 index 000000000..e03fb7bcb --- /dev/null +++ b/javatests/google/registry/testing/AppEngineRuleTest.java @@ -0,0 +1,95 @@ +// Copyright 2019 The Nomulus 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 google.registry.testing; + +import static google.registry.testing.JUnitBackports.assertThrows; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.base.Joiner; +import java.io.File; +import java.io.IOException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Unit tests for {@link google.registry.testing.AppEngineRule}. Tests focus on Datastoe-related + * assertions made during teardown checks. + */ +@RunWith(JUnit4.class) +public class AppEngineRuleTest { + + // An arbitrary index in google/registry/env/common/default/WEB-INF/datastore-indexes.xml + private static final String DECLARED_INDEX = + Joiner.on('\n') + .join( + "", + " ", + " ", + " ", + " ", + " ", + ""); + private static final String UNDECLARED_INDEX = + DECLARED_INDEX.replace("ContactResource", "NoSuchResource"); + + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private final AppEngineRule appEngineRule = AppEngineRule.builder().withDatastore().build(); + + @Before + public void setupAppEngineRule() throws Exception { + appEngineRule.temporaryFolder = temporaryFolder; + appEngineRule.before(); + } + + @Test + public void testTeardown_successNoAutoIndexFile() { + appEngineRule.after(); + } + + @Test + public void testTeardown_successEmptyAutoIndexFile() throws Exception { + writeAutoIndexFile(""); + appEngineRule.after(); + } + + @Test + public void testTeardown_successWhiteSpacesOnlyAutoIndexFile() throws Exception { + writeAutoIndexFile(" "); + appEngineRule.after(); + } + + @Test + public void testTeardown_successOnlyDeclaredIndexesUsed() throws Exception { + writeAutoIndexFile(DECLARED_INDEX); + appEngineRule.after(); + } + + @Test + public void testTeardown_failureUndeclaredIndexesUsed() throws Exception { + writeAutoIndexFile(UNDECLARED_INDEX); + assertThrows(AssertionError.class, () -> appEngineRule.after()); + } + + private void writeAutoIndexFile(String content) throws IOException { + com.google.common.io.Files.asCharSink( + new File(temporaryFolder.getRoot(), "datastore-indexes-auto.xml"), UTF_8) + .write(content); + } +} diff --git a/javatests/google/registry/testing/BUILD b/javatests/google/registry/testing/BUILD index 906f6d0a1..57e86c55a 100644 --- a/javatests/google/registry/testing/BUILD +++ b/javatests/google/registry/testing/BUILD @@ -5,10 +5,15 @@ package( licenses(["notice"]) # Apache 2.0 +load("//java/com/google/testing/builddefs:GenTestRules.bzl", "GenTestRules") + +TEST_CLASSES = ["AppEngineRuleTest.java"] + java_library( name = "testing", srcs = glob( ["*.java"], + exclude = TEST_CLASSES, ), resources = [ "logging.properties", @@ -57,3 +62,22 @@ java_library( "@org_mockito_all", ], ) + +java_library( + name = "tests", + srcs = TEST_CLASSES, + deps = [ + ":testing", + "@com_google_guava", + "@junit", + ], +) + +GenTestRules( + name = "GeneratedTestRules", + default_test_size = "small", + test_files = glob(["*Test.java"]), + deps = [ + ":tests", + ], +)