[
https://issues.apache.org/jira/browse/GEODE-8173?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126982#comment-17126982
]
ASF GitHub Bot commented on GEODE-8173:
---------------------------------------
kirklund commented on a change in pull request #5208:
URL: https://github.com/apache/geode/pull/5208#discussion_r436075685
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -10269,4 +10269,8 @@ boolean hasAnyClientsInterested() {
return (getRegionAdvisor().hasPRServerWithInterest()
|| getRegionAdvisor().hasPRServerWithCQs());
}
+
+ boolean isTransactionDistributed() {
Review comment:
I hate the idea of adding methods like this to Regions. It has nothing
to do with Region. Maybe Transaction or TransactionManager would be a better
place for this method.
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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 org.apache.geode.internal.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+ private PartitionedRegionClear partitionedRegionClear;
+ private DistributionManager distributionManager;
+ private PartitionedRegion partitionedRegion;
+
+ @Before
+ public void setUp() {
+
+ partitionedRegion = mock(PartitionedRegion.class);
+ distributionManager = mock(DistributionManager.class);
+
+
when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+ when(partitionedRegion.getName()).thenReturn("prRegion");
+
+ partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+ }
+
+ private Set<BucketRegion> setupBucketRegions(
+ PartitionedRegionDataStore partitionedRegionDataStore,
+ BucketAdvisor bucketAdvisor) {
+ final int numBuckets = 2;
+ HashSet<BucketRegion> bucketRegions = new HashSet<>();
Review comment:
Let's move away from the Jason Penney quirks. We should be declaring
vars as the interface instead of the impl:
```
Set<BucketRegion> bucketRegions = new HashSet<>();
```
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/cache/ReplicateCacheListenerDistributedTest.java
##########
@@ -179,7 +180,13 @@ public void afterRegionDestroyIsInvokedInEveryMember() {
region.destroyRegion();
-
assertThat(sharedCountersRule.getTotal(REGION_DESTROY)).isEqualTo(expectedRegionDestroys());
+ if (region instanceof PartitionedRegion) {
Review comment:
You typically want to avoid if-else blocks like this, and separate it
into two different tests. One for PR and one for Replicate.
The test class is ReplicateCacheListenerDistributedTest. I can't find any
PartitionedRegions being created in this test. Does this belong in
PRCacheListenerDistributedTest instead?
##########
File path:
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionClearTest.java
##########
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You 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 org.apache.geode.internal.cache;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.CancelCriterion;
+import org.apache.geode.cache.PartitionedRegionPartialClearException;
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.distributed.internal.DMStats;
+import org.apache.geode.distributed.internal.DistributionManager;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.MembershipListener;
+import
org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.cache.partitioned.RegionAdvisor;
+import org.apache.geode.internal.cache.versions.RegionVersionVector;
+
+public class PartitionedRegionClearTest {
+
+
+ private PartitionedRegionClear partitionedRegionClear;
+ private DistributionManager distributionManager;
+ private PartitionedRegion partitionedRegion;
+
+ @Before
+ public void setUp() {
+
+ partitionedRegion = mock(PartitionedRegion.class);
+ distributionManager = mock(DistributionManager.class);
+
+
when(partitionedRegion.getDistributionManager()).thenReturn(distributionManager);
+ when(partitionedRegion.getName()).thenReturn("prRegion");
+
+ partitionedRegionClear = new PartitionedRegionClear(partitionedRegion);
+ }
+
+ private Set<BucketRegion> setupBucketRegions(
+ PartitionedRegionDataStore partitionedRegionDataStore,
+ BucketAdvisor bucketAdvisor) {
+ final int numBuckets = 2;
+ HashSet<BucketRegion> bucketRegions = new HashSet<>();
Review comment:
We should be declaring vars as the interface instead of the impl:
```
Set<BucketRegion> bucketRegions = new HashSet<>();
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Add test coverage for PartitionedRegionClear class
> --------------------------------------------------
>
> Key: GEODE-8173
> URL: https://issues.apache.org/jira/browse/GEODE-8173
> Project: Geode
> Issue Type: Bug
> Components: regions
> Reporter: Anilkumar Gingade
> Priority: Major
> Labels: GeodeOperationAPI
> Fix For: 1.14.0
>
>
> Add test coverage for PartitionedRegionClear class.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
