DonalEvans commented on a change in pull request #7107:
URL: https://github.com/apache/geode/pull/7107#discussion_r749772448
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/ProxyBucketRegionTest.java
##########
@@ -32,4 +46,54 @@ public void shouldBeMockable() throws Exception {
assertThat(mockProxyBucketRegion.getBucketAdvisor()).isSameAs(mockBucketAdvisor);
}
+
+ @Test
+ public void testRecoverFromDisk() throws Exception {
Review comment:
This test name needs to be more descriptive. It should say what
behaviour is under test, what the conditions are, and what the expected
behaviour is. Also, an exception is never thrown from this method, so this can
be removed.
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/ProxyBucketRegionTest.java
##########
@@ -32,4 +46,54 @@ public void shouldBeMockable() throws Exception {
assertThat(mockProxyBucketRegion.getBucketAdvisor()).isSameAs(mockBucketAdvisor);
}
+
+ @Test
+ public void testRecoverFromDisk() throws Exception {
+ PartitionedRegion partitionedRegion = mock(PartitionedRegion.class);
+ InternalRegionArguments internalRegionArguments =
mock(InternalRegionArguments.class);
+ RegionAdvisor regionAdvisor = mock(RegionAdvisor.class);
+ PartitionAttributes partitionAttributes = mock(PartitionAttributes.class);
+ InternalCache cache = mock(InternalCache.class);
+ InternalDistributedSystem ids = mock(InternalDistributedSystem.class);
+ DataPolicy dp = mock(DataPolicy.class);
+ RegionAttributes ra = mock(RegionAttributes.class);
+ DiskStoreImpl ds = mock(DiskStoreImpl.class);
+ DiskInitFile dif = mock(DiskInitFile.class);
+ DiskRegion dr = mock(DiskRegion.class);
+ DistributionManager dm = mock(DistributionManager.class);
+ DistributionConfig config = mock(DistributionConfig.class);
+ CancelCriterion cancel = mock(CancelCriterion.class);
+
+
when(internalRegionArguments.getPartitionedRegionAdvisor()).thenReturn(regionAdvisor);
+ when(regionAdvisor.getPartitionedRegion()).thenReturn(partitionedRegion);
+
when(partitionedRegion.getPartitionAttributes()).thenReturn(partitionAttributes);
+ when(partitionAttributes.getColocatedWith()).thenReturn(null);
+ when(partitionedRegion.getCache()).thenReturn(cache);
+ when(partitionedRegion.getGemFireCache()).thenReturn(cache);
+ when(cache.getInternalDistributedSystem()).thenReturn(ids);
+ when(ids.getDistributionManager()).thenReturn(dm);
+ when(partitionedRegion.getDataPolicy()).thenReturn(dp);
+ when(dp.withPersistence()).thenReturn(true);
+ when(cache.getInternalDistributedSystem()).thenReturn(ids);
+ when(partitionedRegion.getAttributes()).thenReturn(ra);
+ when(ra.getEvictionAttributes()).thenReturn(null);
+ when(partitionedRegion.getDiskStore()).thenReturn(ds);
+ when(ds.getDiskInitFile()).thenReturn(dif);
+ when(dif.createDiskRegion(any(), anyString(), anyBoolean(), anyBoolean(),
anyBoolean(),
+ anyBoolean(), any(), any(), any(), any(), any(), anyString(),
anyInt(), any(),
+ anyBoolean())).thenReturn(dr);
+ when(dm.getConfig()).thenReturn(config);
+ when(config.getAckWaitThreshold()).thenReturn(10);
+ when(cache.getCancelCriterion()).thenReturn(cancel);
+ when(regionAdvisor.isInitialized()).thenReturn(true);
+
+ ProxyBucketRegion proxyBucketRegion =
+ new ProxyBucketRegion(0, partitionedRegion, internalRegionArguments);
+
+ proxyBucketRegion.recoverFromDisk();
+ proxyBucketRegion.recoverFromDisk();
+ verify(regionAdvisor, times(1))
+ .isInitialized();
Review comment:
It's not at all clear to me what this is verifying, and this test passes
even with the changes in `ProxyBucketRegion` reverted, so it doesn't seem to be
testing the new behaviour. I feel like for this change it might be good to add
an integration test that shows the changed behaviour at a higher level, by
actually creating a colocated region and showing that already recovered buckets
are not recovered again.
--
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]
