> On May 24, 2016, 10:18 a.m., Ken Howe wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/LocalDataSetTest.java, > > line 37 > > <https://reviews.apache.org/r/47748/diff/1/?file=1392002#file1392002line37> > > > > Is it necesary to mock pr.isEmpty()? With the change in > > LocalDataSet.isEmpty I would think that calling PartionedRegion.isEmpty > > should cause test failures in both of these new tests. So, something like > > this: > > > > verify(pr, never()).isEmpty()
I started out with wanting the unit test to fail with the old implementation which only used pr.isEmpty() to determine if the localDataSet was empty. Then I wanted to confirm that the new implementation would honor what pr.entryCount(Set) returns even if it differed from pr.isEmpty(). You are correct that I could have instead said "verify that localDataSet.isEmpty never calls pr.isEmpty". Its not clear to me if that is better. I prefer the current unit test because it does not say "you can never call pr.isEmpty". It just says "you answer needs to be consistent with pr.entryCount(Set) and not pr.isEmpty". - Darrel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47748/#review134604 ----------------------------------------------------------- On May 24, 2016, 9:22 a.m., Darrel Schneider wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47748/ > ----------------------------------------------------------- > > (Updated May 24, 2016, 9:22 a.m.) > > > Review request for geode, Scott Jewell and Ken Howe. > > > Bugs: GEODE-1424 > https://issues.apache.org/jira/browse/GEODE-1424 > > > Repository: geode > > > Description > ------- > > change isEmpty to use size > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalDataSet.java > d8e48e13ea019b19e508ad9987c2789918a14189 > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/LocalDataSetTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/47748/diff/ > > > Testing > ------- > > precheckin > > > Thanks, > > Darrel Schneider > >
