DonalEvans commented on code in PR #6976:
URL: https://github.com/apache/geode/pull/6976#discussion_r872587798
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java:
##########
@@ -85,21 +86,22 @@
import org.apache.geode.internal.cache.BucketAdvisor.ServerBucketProfile;
import
org.apache.geode.internal.cache.execute.InternalFunctionInvocationTargetException;
import org.apache.geode.internal.cache.execute.util.TypedFunctionService;
+import org.apache.geode.logging.internal.log4j.api.LogService;
import org.apache.geode.management.ManagementService;
import org.apache.geode.management.membership.MembershipEvent;
import
org.apache.geode.management.membership.UniversalMembershipListenerAdapter;
import org.apache.geode.test.dunit.AsyncInvocation;
import org.apache.geode.test.dunit.DUnitEnv;
+import org.apache.geode.test.dunit.SerializableCallableIF;
import org.apache.geode.test.dunit.VM;
import org.apache.geode.test.dunit.rules.DistributedRestoreSystemProperties;
import org.apache.geode.test.dunit.rules.DistributedRule;
import org.apache.geode.test.junit.categories.ClientServerTest;
import
org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
@Category(ClientServerTest.class)
-@SuppressWarnings("serial")
public class PartitionedRegionSingleHopDUnitTest implements Serializable {
-
+ protected static Logger logger = LogService.getLogger();
Review Comment:
This logger is not used in the test so should be removed.
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java:
##########
@@ -919,21 +897,22 @@ public void testClientMetadataForPersistentPrs() throws
Exception {
int locatorPort = DUnitEnv.get().getLocatorPort();
- vm0.invoke(() -> createServer("disk", -1, 3, 4));
- vm1.invoke(() -> createServer("disk", -1, 3, 4));
- vm2.invoke(() -> createServer("disk", -1, 3, 4));
- vm3.invoke(() -> createServer("disk", -1, 3, 4));
+ vm0.invoke((SerializableCallableIF<Integer>) this::createServer);
+ vm1.invoke((SerializableCallableIF<Integer>) this::createServer);
+ vm2.invoke((SerializableCallableIF<Integer>) this::createServer);
+ vm3.invoke((SerializableCallableIF<Integer>) this::createServer);
Review Comment:
These casts can be avoided if you use lambdas here:
```
vm0.invoke(() -> createServer());
vm1.invoke(() -> createServer());
vm2.invoke(() -> createServer());
vm3.invoke(() -> createServer());
```
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java:
##########
@@ -1501,11 +1482,11 @@ public String getId() {
}
@Override
- public void execute(FunctionContext context) {
+ public void execute(FunctionContext<T> context) {
RegionFunctionContext rc = (RegionFunctionContext) context;
Region<Object, Object> r = rc.getDataSet();
- Set filter = rc.getFilter();
- if (rc.getFilter() == null) {
+ Set<Object> filter = (Set<Object>) rc.getFilter();
+ if (filter == null) {
for (int i = 0; i < 200; i++) {
r.put(i, i);
}
Review Comment:
The `equals()` method for the Order class below is broken. Line 1604 should
be:
```
if (!(obj instanceof Order)) {
```
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java:
##########
@@ -1405,7 +1385,8 @@ private void waitForLocalBucketsCreation() {
await().untilAsserted(() ->
assertThat(pr.getDataStore().getAllLocalBuckets()).hasSize(4));
Review Comment:
While you're cleaning up the test code, warnings on lines 1328, 1333, 1338
and 1342 can be fixed by using `PutFunction<>()`.
##########
geode-core/src/distributedTest/java/org/apache/geode/cache30/PutAllMultiVmDUnitTest.java:
##########
@@ -486,24 +443,24 @@ public void run2() throws CacheException {
// do nothing
Review Comment:
On line 441 above this, rather than using `fail()` when we expect a call to
raise a throwable but it doesn't, it would be better to use
`assertThatThrownBy()` to assert that we see the expected exception. This
provides a more descriptive failure in the event that we don't see the expected
exception. This comment also applies to the other 4 uses of `fail()` in this
test.
##########
geode-core/src/distributedTest/java/org/apache/geode/cache30/PutAllMultiVmDUnitTest.java:
##########
@@ -347,22 +319,12 @@ public void testPutAllFromPreload() throws
InterruptedException {
@Test
public void testSimplePutAll() {
- Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
- SerializableRunnable clear = new CacheSerializableRunnable("clear") {
- @Override
- public void run2() throws CacheException {
- try {
- region.clear();
- } catch (Exception ex) {
- ex.printStackTrace();
- }
- }
- };// end of clear
+ // end of clear
- vm0.invoke(new CacheSerializableRunnable("testSimplePutAll1") {
+ vm0.invoke("testSimplePutAll1", new CacheSerializableRunnable() {
Review Comment:
This (and other uses of `CacheSerializableRunnable` below) can be simplified
by using a lambda instead:
```
vm0.invoke("testSimplePutAll1", () -> {
int cntr = 0, cntr1 = 0;
for (int i = 1; i < 6; i++) {
region.put(i, "testSimplePutAll" + i);
cntr++;
}
...
});
```
##########
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionSingleHopDUnitTest.java:
##########
@@ -956,16 +935,16 @@ public void testClientMetadataForPersistentPrs() throws
Exception {
});
}
- LATCH.get().await(getTimeout().toMillis(), MILLISECONDS);
-
+ assertThat(LATCH.get().await(getTimeout().toMillis(), MILLISECONDS))
+ .as("Waiting for the latch").isTrue();
AsyncInvocation<Integer> createServerOnVM3 =
- vm3.invokeAsync(() -> createServer("disk", -1, 3, 4));
+ vm3.invokeAsync((SerializableCallableIF<Integer>) this::createServer);
AsyncInvocation<Integer> createServerOnVM2 =
- vm2.invokeAsync(() -> createServer("disk", -1, 3, 4));
+ vm2.invokeAsync((SerializableCallableIF<Integer>) this::createServer);
AsyncInvocation<Integer> createServerOnVM1 =
- vm1.invokeAsync(() -> createServer("disk", -1, 3, 4));
+ vm1.invokeAsync((SerializableCallableIF<Integer>) this::createServer);
AsyncInvocation<Integer> createServerOnVM0 =
- vm0.invokeAsync(() -> createServer("disk", -1, 3, 4));
+ vm0.invokeAsync((SerializableCallableIF<Integer>) this::createServer);
Review Comment:
More casts that can be avoided by using lambdas rather than method
references.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
