----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46660/#review130508 -----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tx/AbstractPeerTXRegionStub.java (line 50) <https://reviews.apache.org/r/46660/#comment194272> I would not use the LocalizedString here. It does not fix since it is not a PartitionMessage. I also wouldn't bother adding a new LocalizedString. We have GEODE-536 saying we should remove it. So just use a string literal like "Cache was closed while fetching keys". geode-core/src/test/java/com/gemstone/gemfire/internal/cache/tx/AbstractPeerTXRegionStubTest.java (line 128) <https://reviews.apache.org/r/46660/#comment194271> Why is this doReturn needed? Since state is mocked in setUp I think all its methods will return defaults (in this case null). So why explicitly say that state.getTarget() should return null? If you do need this would it be better to have this a part in setUp since it is not specific to this test method? - Darrel Schneider On April 25, 2016, 2:43 p.m., Ken Howe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46660/ > ----------------------------------------------------------- > > (Updated April 25, 2016, 2:43 p.m.) > > > Review request for geode, Darrel Schneider, Sai Boorlagadda, and Swapnil > Bawaskar. > > > Repository: geode > > > Description > ------- > > Catch the exception and throw TransactionDataNodeHasDepartedException to the > application. > There were no existing unit tests for AbstractPeerTxRegionStub, so so one was > added for getRegionKeysForIteration, which overrides the method from > TxRegionStub interface. > > Refactored AbstractRegion.getSystem - removed final keyword to allow > subclassing for testing. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegion.java > 10644cb8f661004dcd35351201ee97044baa936a > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tx/AbstractPeerTXRegionStub.java > 77116eb8a8b9a97fd24c58554f54e4d59c61e3e4 > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/tx/AbstractPeerTXRegionStubTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/46660/diff/ > > > Testing > ------- > > precheckin run with no new failures > > > Thanks, > > Ken Howe > >
