Some steps I typically follow in cleaning up the test:

1) Rename the test
Commit the file rename as its own commit

2) Move all constants to top of test class
Move all fields to top of test class
Move all inner-classes to bottom of test class
Move all Rules below fields
Move all setUp/tearDown methods below Rules
Move all test methods below setUp/tearDown methods
Move all other methods below test methods

All fields, vars, etc should be defined alone on a dedicated line

3) Reduce scope of everything to small as possible (ex: private)
* Rules must be defined public
* Mockito @Mock fields can be defined private

4) Update use of deprecated APIs with current Geode APIs

5) Convert JUnit assertions to AssertJ
* I use regex find/replace in IntelliJ
* See https://joel-costigliola.github.io/assertj/assertj-core-converting-junit-assertions-to-assertj.html
* Review them and adjust as needed
* Convert if-then-fail blocks to use assertions

6) Convert multiple variations of the same test to JUnitParams
(Usually two tests both calling same doTest method with one or more parameters)

* Add @RunWith(JUnitParamsRunner.class) at top of test class
* Adjust one or more test method to use JUnitParams:

From:
@Test
public void test1CCEServer() {
doServerTest(true);
}
@Test
public void test2NormalServer() {
doServerTest(false);
}
private void doServerTest(boolean CCE) {
...
}

To:
@Test
@Parameters({"true", "false"})
@TestCaseName("{method}(concurrencyChecksEnabled={0})")
public void clientReceivesKeysRegisteredFor(boolean concurrencyChecksEnabled) {
...
}

7) Change from using helper layers to directly using Geode APIs
* Replace rules and helpers with direct usage of Geode API

8) Rename AsyncInvocation instances to indicate what and where

AsyncInvocation createCqInClient1 = client1.invokeAsync

Notes:
* what = create continuous query (cq)
* where = in client1

9) Use AsyncInvocation.await() instead of .join(), .exceptionOccurred, or .getException()

createCqInClient1.await()

Notes:
* does all of the above AND uses the GeodeAwaitility timeout

10) Convert from WaitCriterion with assertion to Awaitility (GeodeAwaitility) with assertion

11) Convert from non-assertion WaitCriterion to Awaitility (GeodeAwaitility)

From:
WaitCriterion ev = new WaitCriterion() {
@Override
public boolean done() {
Entry entry = region.getEntry(key);
return entry != null && entry.getValue() == null;
}

@Override
public String description() {
return null;
}
};
Wait.waitForCriterion(ev, 10 * 1000, 1000, true);

To:
await("entry with null value exists for " + key).until(() -> {
Entry entry = region.getEntry(key);
return entry != null && entry.getValue() == null;
});

Or:
await("entry with null value exists for " + key).untilAsserted(() -> {
assertThat(region.getEntry(key).getValue()).isNull();
});

Notes:
* untilAsserted catches NPEs and keeps retrying until the timeout

12) Inline super-class(es) into current test class without deleting super-class(es)
* Delete everything that is not used (repeat until done)
* Inline all super classes except DistributedTestCase or CacheTestCase if those are used

13) Inline helper methods (especially small ones)

From:
public void myTest() {
closeCache();
...
private void closeCache() {
vm.invoke(() -> getCache().close());
}

To:
public void myTest() {
vm.invoke(() -> getCache().close());

14) Transpose VM parameters on a method to just having the VM invoke the method

From:
public void myTest() {
createCache(server2);
...
private void createCache(VM vm) {
vm.invoke(() -> {
// many lines to create cache, diskstores, regions, etc.
});
}

To:
public void myTest() {
server2.invoke(() -> createCache());
...
private void createCache() {
// many lines to create cache, diskstores, regions, etc.
}

15) Remove all catch-blocks and fail statements for unexpected exceptions
* Delete the try/catch with fail/throw
* Add all checked exceptions to method signatures
* Do not use "throws Throwable" or "throws Error" in tests

From:
public/private void method() {
...
try {
// do something that may throw checked exception
} catch (Exception e) {
fail(e);
// or:
throw new AssertionError(e);
}
...
}

To:
public/private void method() throws IOException {
...
// do something that may throw checked exception
...
}

16) Replace try-fail-catch pattern for expected exception with catchThrowable or assertThatThrownBy

From:
try {
doPutAll(regionName, title, numberOfEntries);
fail("expected to throw");
} catch (ServerOperationException e) {
assertTrue(e.getMessage().contains("putAll at server applied partial keys due to exception"));
}

To:
Throwable thrown = catchThrowable(() -> doPutAll(regionName, title, numberOfEntries));
assertThat(thrown)
.isInstanceOf(ServerOperationException.class)
.hasMessageContaining("putAll at server applied partial keys due to exception");

17) Use description test method naming

18) Remove atMost from uses of GeodeAwaitility to ensure full timeout of 5 minutes is used

19) Update or fix object wait/notify usage
* Awaitility is usually much better

20) Change thread-unsafe callback (listener) fields to volatile or Atomic instances

21) Avoid use of assertThat(region).hasSize(n)

Do use this syntax for EVERY collection type except for Region.

AssertJ treats Region as a map and will walk it to generate the failure message if
the assertion fails. This can result in messaging and even hangs, especially if the type
is a PartitionedRegion.

Examples that are ok:
assertThat(region.getSize()).isEqualTo(10);
assertThat(region.getSize()).isGreaterThanOrEqualTo(1);

22) Sometimes transposing VM invoke with assertion can look or work better one way or the other

From:
assertThat(getRegionSize(server1, regionName)).isEqualTo(0);

To:
server1.invoke(() -> {
assertThat(getRegionSize(regionName)).isEqualTo(0);
});

Or:
server1.invoke(() -> {
assertThat(getCache().getRegion(regionName).size()).as("region size").isEqualTo(0);
});

Or:
assertThat(server1.invoke(() -> getCache().getRegion(regionName).size()).isEqualTo(0);

Or (this is typically what I do):
server1.invoke(() -> {
assertThat(getCache().getRegion(regionName).size()).as("region size").isZero();
});

23) Convert SerializableRunnable/SerializableCallable fields to methods and invoke from lambda

From:
private SerializableRunnable doSomethingInteresting = new SerializableRunnable("doSomethingInteresting") {
@Override
public void run() {
// do something
}
};

clientVM.invoke(doSomethingInteresting);

To:
private void doSomethingInteresting() {
// do something
}

clientVM.invoke(() -> doSomethingInteresting());

  • No labels