kirklund commented on a change in pull request #6104:
URL: https://github.com/apache/geode/pull/6104#discussion_r590666674
##########
File path:
geode-cq/src/distributedTest/java/org/apache/geode/cache/query/cq/dunit/PartitionedRegionCqQueryDUnitTest.java
##########
@@ -120,6 +123,60 @@
private static int bridgeServerPort;
+ @Test
+ public void testPutAllWithCQLocalDestroy() throws Exception {
+ VM server1 = getVM(0);
+ VM server2 = getVM(1);
+ VM client = getVM(2);
+
+ final String cqName = "testPutAllWithCQLocalDestroy_0";
+ createServer(server1);
+ createServer(server2);
+ final String host = Host.getHost(0).getHostName();
+ final int port = server2.invoke(() ->
PartitionedRegionCqQueryDUnitTest.getCacheServerPort());
+ createClient(client, port, host);
+ createCQ(client, cqName, cqs[0]);
+
+ int numObjects = 1000;
+
+ server1.invoke(() -> {
+ Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR +
regions[0]);
+ Map buffer = new HashMap();
+ for (int i = 1; i < numObjects; i++) {
+ Portfolio p = new Portfolio(i);
+ buffer.put("" + i, p);
+ }
+ region.putAll(buffer);
+ });
+
+ client.invoke(() -> {
+ QueryService cqService = getCache().getQueryService();
+ CqQuery cqQuery = cqService.getCq(cqName);
+ if (cqQuery == null) {
+ fail("Failed to get CQ " + cqName);
+ }
+ try {
+ cqQuery.executeWithInitialResults();
+ } catch (Exception ex) {
+ fail("Failed to execute CQ " + cqName, ex);
+ }
+ });
+
+ server1.invoke(() -> {
+ Region region = getCache().getRegion(SEPARATOR + "root" + SEPARATOR +
regions[0]);
+ Map buffer = new HashMap();
+ for (int i = 1; i < numObjects; i++) {
+ Portfolio p = new Portfolio(-1 * i);
+ buffer.put("" + i, p);
+ }
+ region.putAll(buffer);
+ });
+
+ cqHelper.closeClient(client);
+ cqHelper.closeServer(server2);
+ cqHelper.closeServer(server1);
Review comment:
It's better (and more industry standard for writing JUnit tests) if you
can figure out a way to move cleanup code from the end of the test method to a
`tearDown` method annotated with `@After`. For example, if you make
`cqHelper.closeClient` return without error if client is null or not running,
then you could hoist these local variables to fields and the test would look
more like this:
```
private VM client;
private VM server1;
private VM server2;
@Before
public void setUp() {
server1 = getVM(0);
server2 = getVM(1);
client = getVM(2);
}
@After
public void tearDown() {
cqHelper.closeClient(client);
cqHelper.closeServer(server2);
cqHelper.closeServer(server1);
}
```
And `cqHelper` close methods could have some early-outs like this:
```
public void closeClient(VM vm) {
if (vm == null) {
return;
}
// etc
}
```
----------------------------------------------------------------
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:
[email protected]
