kirklund commented on a change in pull request #6964:
URL: https://github.com/apache/geode/pull/6964#discussion_r727404430
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -5602,6 +5625,12 @@ public void cleanupFailedInitialization() {
if (logger.isDebugEnabled()) {
logger.debug("cleanupFailedInitialization: end of {}", getName());
}
+ if (savedFirstRuntimeException != null
+ && savedFirstRuntimeException instanceof
DistributedSystemDisconnectedException) {
+ logger.warn("cleanupFailedInitialization originally failed with ",
+ savedFirstRuntimeException);
Review comment:
I think you should probably use this syntax:
```
logger.warn("cleanupFailedInitialization originally failed with {}",
savedFirstRuntimeException, savedFirstRuntimeException);
```
If you don't want it to print the stack trace, then remove the 2nd
savedFirstRuntimeException.
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTest.java
##########
@@ -443,6 +449,38 @@ public void
generatePRIdShouldNotThrowNumberFormatExceptionIfAnErrorOccursWhileR
.doesNotThrowAnyException();
}
+ @Test
+ public void
registerPartitionedRegionShouldHandleLockServiceDestroyedException()
+ throws ClassNotFoundException {
+ AttributesFactory attributesFactory = new AttributesFactory();
+ attributesFactory.setPartitionAttributes(
+ new
PartitionAttributesFactory().setTotalNumBuckets(1).setRedundantCopies(1)
+ .setLocalMaxMemory(0).create());
+
+ partitionedRegion = new PartitionedRegion("region",
attributesFactory.create(), null, cache,
+ mock(InternalRegionArguments.class), disabledClock(),
ColocationLoggerFactory.create());
+
+ PartitionedRegion spyPartitionedRegion = spy(partitionedRegion);
+
+ InternalDistributedMember imageTarget =
mock(InternalDistributedMember.class);
+ InternalRegionFactory factory = mock(InternalRegionFactory.class);
+
when(cache.createInternalRegionFactory(RegionShortcut.REPLICATE)).thenReturn(factory);
+ DistributedRegion partitionedRegionRoot = mock(DistributedRegion.class);
+
when(factory.create(PR_ROOT_REGION_NAME)).thenReturn(partitionedRegionRoot);
+ CacheDistributionAdvisor cda = mock(CacheDistributionAdvisor.class);
+ doNothing().when(cda).addMembershipListener(any());
+ when(partitionedRegionRoot.getDistributionAdvisor()).thenReturn(cda);
+ PartitionedRegion.RegionLock regionLock =
mock(PartitionedRegion.RegionLock.class);
+ doReturn(regionLock).when(spyPartitionedRegion).getRegionLock();
+ doThrow(new LockServiceDestroyedException("Lock Service is destroyed in
test")).when(regionLock)
+ .lock();
+ doThrow(new
DistributedSystemDisconnectedException("test")).when(spyPartitionedRegion)
+ .cleanupFailedInitialization();
+ assertThatThrownBy(() -> spyPartitionedRegion.initialize(null,
imageTarget, null))
+ .isInstanceOf(PartitionedRegionException.class)
+ .hasCauseInstanceOf(DistributedSystemDisconnectedException.class);
+ }
+
Review comment:
Tests can get hard to read when the mocking and stubbing gets this big.
It's still ok as is, but you might want to try ordering the test with all of
the mocks first (top of test method), then all of the stubbings, then a blank
line before the call to spyPartitionedRegion.initialize.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
