kirklund commented on a change in pull request #6008:
URL: https://github.com/apache/geode/pull/6008#discussion_r573344449
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegion.java
##########
@@ -741,8 +741,8 @@ public boolean isPdxTypesRegion() {
// This is for all regions except pdx Region
if (!isPdxTypesRegion) {
// Make sure we are distributing to only those senders whose id
- // is available on this region
- if (allGatewaySenderIds.contains(sender.getId())) {
+ // is available on this region and whose state is running
+ if (allGatewaySenderIds.contains(sender.getId()) &&
sender.isRunning()) {
Review comment:
Please consider writing a unit test for any code that only has
integration/distributed test coverage. The following example is using a basic
technique from Working Effectively with Legacy Code.
For example, you could create a new method called:
```
@VisibleForTesting
static boolean hasRunningGatewaySender(Set<GatewaySender> senders,
GatewaySender sender) {
return senders.contains(sender) && sender.isRunning();
}
```
And then you can add unit tests for this new method to `AbstractRegionTest`:
```
import java.util.Collections;
import org.apache.geode.cache.wan.GatewaySender;
@Test
public void hasRunningGatewaySender_returnsFalse_ifSendersIsEmpty() {
GatewaySender sender = mock(GatewaySender.class);
boolean value =
AbstractRegion.hasRunningGatewaySender(Collections.emptySet(), sender);
assertThat(value).isFalse();
}
```
You can also write it as a non-static method, but the unit tests will
require a bit more setUp.
----------------------------------------------------------------
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]
