zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-08-11 20:19:37 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix flaky test issues caused by lack of ofy init (#1246)

Browse source
This commit is contained in:
gbrodman 2021-07-20 13:14:41 -04:00 • committed by GitHub
parent 6849bf6914
commit 6ec2e9501d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 42 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

10
core/src/test/java/google/registry/beam/rde/RdePipelineTest.java
View file

@ -23,7 +23,6 @@ import static google.registry.model.common.Cursor.CursorType.RDE_STAGING;
import static google.registry.model.rde.RdeMode.FULL; import static google.registry.model.rde.RdeMode.FULL;
import static google.registry.model.rde.RdeMode.THIN; import static google.registry.model.rde.RdeMode.THIN;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.persistence.transaction.TransactionManagerFactory.setTmForTest;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.rde.RdeResourceType.CONTACT; import static google.registry.rde.RdeResourceType.CONTACT;
import static google.registry.rde.RdeResourceType.DOMAIN; import static google.registry.rde.RdeResourceType.DOMAIN;
@ -50,7 +49,7 @@ import google.registry.model.registrar.Registrar;
import google.registry.model.registrar.Registrar.State; import google.registry.model.registrar.Registrar.State;
import google.registry.persistence.transaction.JpaTestRules; import google.registry.persistence.transaction.JpaTestRules;
import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension; import google.registry.persistence.transaction.JpaTestRules.JpaIntegrationTestExtension;
import google.registry.persistence.transaction.TransactionManager; import google.registry.persistence.transaction.TransactionManagerFactory;
import google.registry.rde.DepositFragment; import google.registry.rde.DepositFragment;
import google.registry.rde.PendingDeposit; import google.registry.rde.PendingDeposit;
import google.registry.rde.RdeResourceType; import google.registry.rde.RdeResourceType;
@ -117,12 +116,9 @@ public class RdePipelineTest {
private RdePipeline rdePipeline; private RdePipeline rdePipeline;
private TransactionManager originalTm;
@BeforeEach @BeforeEach
void beforeEach() throws Exception { void beforeEach() throws Exception {
originalTm = tm(); TransactionManagerFactory.setTmForTest(jpaTm());
setTmForTest(jpaTm());
loadInitialData(); loadInitialData();
// Two real registrars have been created by loadInitialData(), named "New Registrar" and "The // Two real registrars have been created by loadInitialData(), named "New Registrar" and "The
@ -169,7 +165,7 @@ public class RdePipelineTest {
@AfterEach @AfterEach
void afterEach() { void afterEach() {
setTmForTest(originalTm); TransactionManagerFactory.removeTmOverrideForTest();
} }
@Test @Test

28
core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java
View file

@ -17,7 +17,7 @@ package google.registry.persistence.transaction;
import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm; import static google.registry.persistence.transaction.TransactionManagerFactory.jpaTm;
import static google.registry.testing.DatabaseHelper.assertDetached; import static google.registry.testing.DatabaseHelper.assertDetachedFromEntityManager;
import static google.registry.testing.TestDataHelper.fileClassPath; import static google.registry.testing.TestDataHelper.fileClassPath;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
@ -388,7 +388,8 @@ class JpaTransactionManagerImplTest {
void load_succeeds() { void load_succeeds() {
assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse(); assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse();
jpaTm().transact(() -> jpaTm().insert(theEntity)); jpaTm().transact(() -> jpaTm().insert(theEntity));
TestEntity persisted = jpaTm().transact(() -> assertDetached(jpaTm().loadByKey(theEntityKey))); TestEntity persisted =
jpaTm().transact(() -> assertDetachedFromEntityManager(jpaTm().loadByKey(theEntityKey)));
assertThat(persisted.name).isEqualTo("theEntity"); assertThat(persisted.name).isEqualTo("theEntity");
assertThat(persisted.data).isEqualTo("foo"); assertThat(persisted.data).isEqualTo("foo");
} }
@ -404,7 +405,8 @@ class JpaTransactionManagerImplTest {
@Test @Test
void loadByEntity_succeeds() { void loadByEntity_succeeds() {
jpaTm().transact(() -> jpaTm().insert(theEntity)); jpaTm().transact(() -> jpaTm().insert(theEntity));
TestEntity persisted = jpaTm().transact(() -> assertDetached(jpaTm().loadByEntity(theEntity))); TestEntity persisted =
jpaTm().transact(() -> assertDetachedFromEntityManager(jpaTm().loadByEntity(theEntity)));
assertThat(persisted.name).isEqualTo("theEntity"); assertThat(persisted.name).isEqualTo("theEntity");
assertThat(persisted.data).isEqualTo("foo"); assertThat(persisted.data).isEqualTo("foo");
} }
@ -414,7 +416,11 @@ class JpaTransactionManagerImplTest {
assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse(); assertThat(jpaTm().transact(() -> jpaTm().exists(theEntity))).isFalse();
jpaTm().transact(() -> jpaTm().insert(theEntity)); jpaTm().transact(() -> jpaTm().insert(theEntity));
TestEntity persisted = TestEntity persisted =
jpaTm().transact(() -> assertDetached(jpaTm().loadByKeyIfPresent(theEntityKey).get())); jpaTm()
.transact(
() ->
assertDetachedFromEntityManager(
jpaTm().loadByKeyIfPresent(theEntityKey).get()));
assertThat(persisted.name).isEqualTo("theEntity"); assertThat(persisted.name).isEqualTo("theEntity");
assertThat(persisted.data).isEqualTo("foo"); assertThat(persisted.data).isEqualTo("foo");
} }
@ -431,7 +437,9 @@ class JpaTransactionManagerImplTest {
assertThat(jpaTm().transact(() -> jpaTm().exists(compoundIdEntity))).isFalse(); assertThat(jpaTm().transact(() -> jpaTm().exists(compoundIdEntity))).isFalse();
jpaTm().transact(() -> jpaTm().insert(compoundIdEntity)); jpaTm().transact(() -> jpaTm().insert(compoundIdEntity));
TestCompoundIdEntity persisted = TestCompoundIdEntity persisted =
jpaTm().transact(() -> assertDetached(jpaTm().loadByKey(compoundIdEntityKey))); jpaTm()
.transact(
() -> assertDetachedFromEntityManager(jpaTm().loadByKey(compoundIdEntityKey)));
assertThat(persisted.name).isEqualTo("compoundIdEntity"); assertThat(persisted.name).isEqualTo("compoundIdEntity");
assertThat(persisted.age).isEqualTo(10); assertThat(persisted.age).isEqualTo(10);
assertThat(persisted.data).isEqualTo("foo"); assertThat(persisted.data).isEqualTo("foo");
@ -450,7 +458,7 @@ class JpaTransactionManagerImplTest {
theEntityKey, VKey.createSql(TestEntity.class, "does-not-exist"))); theEntityKey, VKey.createSql(TestEntity.class, "does-not-exist")));
assertThat(results).containsExactly(theEntityKey, theEntity); assertThat(results).containsExactly(theEntityKey, theEntity);
assertDetached(results.get(theEntityKey)); assertDetachedFromEntityManager(results.get(theEntityKey));
}); });
} }
@ -463,7 +471,7 @@ class JpaTransactionManagerImplTest {
ImmutableMap<VKey<? extends TestEntity>, TestEntity> results = ImmutableMap<VKey<? extends TestEntity>, TestEntity> results =
jpaTm().loadByKeysIfPresent(ImmutableList.of(theEntityKey)); jpaTm().loadByKeysIfPresent(ImmutableList.of(theEntityKey));
assertThat(results).containsExactly(theEntityKey, theEntity); assertThat(results).containsExactly(theEntityKey, theEntity);
assertDetached(results.get(theEntityKey)); assertDetachedFromEntityManager(results.get(theEntityKey));
}); });
} }
@ -478,7 +486,7 @@ class JpaTransactionManagerImplTest {
.loadByEntitiesIfPresent( .loadByEntitiesIfPresent(
ImmutableList.of(theEntity, new TestEntity("does-not-exist", "bar"))); ImmutableList.of(theEntity, new TestEntity("does-not-exist", "bar")));
assertThat(results).containsExactly(theEntity); assertThat(results).containsExactly(theEntity);
assertDetached(results.get(0)); assertDetachedFromEntityManager(results.get(0));
}); });
} }
@ -491,7 +499,7 @@ class JpaTransactionManagerImplTest {
ImmutableList<TestEntity> results = ImmutableList<TestEntity> results =
jpaTm().loadByEntities(ImmutableList.of(theEntity)); jpaTm().loadByEntities(ImmutableList.of(theEntity));
assertThat(results).containsExactly(theEntity); assertThat(results).containsExactly(theEntity);
assertDetached(results.get(0)); assertDetachedFromEntityManager(results.get(0));
}); });
} }
@ -503,7 +511,7 @@ class JpaTransactionManagerImplTest {
.transact( .transact(
() -> () ->
jpaTm().loadAllOf(TestEntity.class).stream() jpaTm().loadAllOf(TestEntity.class).stream()
.map(DatabaseHelper::assertDetached) .map(DatabaseHelper::assertDetachedFromEntityManager)
.collect(toImmutableList())); .collect(toImmutableList()));
assertThat(persisted).containsExactlyElementsIn(moreEntities); assertThat(persisted).containsExactlyElementsIn(moreEntities);
} }

29
core/src/test/java/google/registry/persistence/transaction/QueryComposerTest.java
View file

@ -77,7 +77,7 @@ public class QueryComposerTest {
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("name", Comparator.GT, "bravo") .where("name", Comparator.GT, "bravo")
.first() .first()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.get())) .get()))
.isEqualTo(charlie); .isEqualTo(charlie);
assertThat( assertThat(
@ -86,7 +86,7 @@ public class QueryComposerTest {
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("name", Comparator.GTE, "charlie") .where("name", Comparator.GTE, "charlie")
.first() .first()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.get())) .get()))
.isEqualTo(charlie); .isEqualTo(charlie);
assertThat( assertThat(
@ -95,7 +95,7 @@ public class QueryComposerTest {
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("name", Comparator.LT, "bravo") .where("name", Comparator.LT, "bravo")
.first() .first()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.get())) .get()))
.isEqualTo(alpha); .isEqualTo(alpha);
assertThat( assertThat(
@ -104,7 +104,7 @@ public class QueryComposerTest {
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("name", Comparator.LTE, "alpha") .where("name", Comparator.LTE, "alpha")
.first() .first()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.get())) .get()))
.isEqualTo(alpha); .isEqualTo(alpha);
} }
@ -125,7 +125,7 @@ public class QueryComposerTest {
assertThat( assertThat(
transactIfJpaTm( transactIfJpaTm(
() -> () ->
DatabaseHelper.assertDetached( QueryComposerTest.assertDetachedIfJpa(
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("name", Comparator.EQ, "alpha") .where("name", Comparator.EQ, "alpha")
.getSingleResult()))) .getSingleResult())))
@ -174,7 +174,7 @@ public class QueryComposerTest {
.createQueryComposer(TestEntity.class) .createQueryComposer(TestEntity.class)
.where("name", Comparator.GT, "alpha") .where("name", Comparator.GT, "alpha")
.stream() .stream()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.collect(toImmutableList()))) .collect(toImmutableList())))
.containsExactly(bravo, charlie); .containsExactly(bravo, charlie);
assertThat( assertThat(
@ -184,7 +184,7 @@ public class QueryComposerTest {
.createQueryComposer(TestEntity.class) .createQueryComposer(TestEntity.class)
.where("name", Comparator.GTE, "bravo") .where("name", Comparator.GTE, "bravo")
.stream() .stream()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.collect(toImmutableList()))) .collect(toImmutableList())))
.containsExactly(bravo, charlie); .containsExactly(bravo, charlie);
assertThat( assertThat(
@ -194,7 +194,7 @@ public class QueryComposerTest {
.createQueryComposer(TestEntity.class) .createQueryComposer(TestEntity.class)
.where("name", Comparator.LT, "charlie") .where("name", Comparator.LT, "charlie")
.stream() .stream()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.collect(toImmutableList()))) .collect(toImmutableList())))
.containsExactly(alpha, bravo); .containsExactly(alpha, bravo);
assertThat( assertThat(
@ -204,7 +204,7 @@ public class QueryComposerTest {
.createQueryComposer(TestEntity.class) .createQueryComposer(TestEntity.class)
.where("name", Comparator.LTE, "bravo") .where("name", Comparator.LTE, "bravo")
.stream() .stream()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.collect(toImmutableList()))) .collect(toImmutableList())))
.containsExactly(alpha, bravo); .containsExactly(alpha, bravo);
} }
@ -228,7 +228,7 @@ public class QueryComposerTest {
tm().createQueryComposer(TestEntity.class) tm().createQueryComposer(TestEntity.class)
.where("val", Comparator.EQ, 2) .where("val", Comparator.EQ, 2)
.first() .first()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.get())) .get()))
.isEqualTo(bravo); .isEqualTo(bravo);
} }
@ -243,7 +243,7 @@ public class QueryComposerTest {
.where("val", Comparator.GT, 1) .where("val", Comparator.GT, 1)
.orderBy("val") .orderBy("val")
.stream() .stream()
.map(DatabaseHelper::assertDetached) .map(QueryComposerTest::assertDetachedIfJpa)
.collect(toImmutableList()))) .collect(toImmutableList())))
.containsExactly(bravo, alpha); .containsExactly(bravo, alpha);
} }
@ -354,6 +354,13 @@ public class QueryComposerTest {
.contains("The LIKE operation is not supported on Datastore."); .contains("The LIKE operation is not supported on Datastore.");
} }
private static <T> T assertDetachedIfJpa(T entity) {
if (!tm().isOfy()) {
return DatabaseHelper.assertDetachedFromEntityManager(entity);
}
return entity;
}
@javax.persistence.Entity @javax.persistence.Entity
@Entity(name = "QueryComposerTestEntity") @Entity(name = "QueryComposerTestEntity")
private static class TestEntity extends ImmutableObject { private static class TestEntity extends ImmutableObject {

6
core/src/test/java/google/registry/testing/DatabaseHelper.java
View file

@ -1351,14 +1351,12 @@ public class DatabaseHelper {
} }
/** /**
* In JPA mode, assert that the given entity is detached from the current entity manager. * Asserts that the given entity is detached from the current JPA entity manager.
* *
* <p>Returns the original entity object. * <p>Returns the original entity object.
*/ */
public static <T> T assertDetached(T entity) { public static <T> T assertDetachedFromEntityManager(T entity) {
if (!tm().isOfy()) {
assertThat(jpaTm().getEntityManager().contains(entity)).isFalse(); assertThat(jpaTm().getEntityManager().contains(entity)).isFalse();
}
return entity; return entity;
} }