> On May 2, 2016, 11:38 p.m., Jason Huynh wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceBase.java, > > line 65 > > <https://reviews.apache.org/r/46917/diff/1/?file=1369317#file1369317line65> > > > > Do we care about the message in the functionexception? > > > > If so, instead of using the expected = FunctionException.class > > annotation, we can do: > > > > //class variable > > public ExpectedException expectedException = ExpectedException.none(); > > > > //in each test > > expectedException.expect(SomeException.class); > > expectedException.expectMessage("Some message");
Good point. I'm not sure if all of these cases need an assertion on the message, but we might want an assertion on the cause. I'll update these tests to use ExpectedException. > On May 2, 2016, 11:38 p.m., Jason Huynh wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemoteRRDUnitTest.java, > > line 18 > > <https://reviews.apache.org/r/46917/diff/1/?file=1369322#file1369322line18> > > > > Should this be proxy RR instead of remote RR? > > > > Is this a client or a peer proxy? PeerProxy. I'll rename the test. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46917/#review131417 ----------------------------------------------------------- On May 2, 2016, 10:57 p.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46917/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 10:57 p.m.) > > > Review request for geode, anilkumar gingade and nabarun nag. > > > Repository: geode > > > Description > ------- > > Testing how functions propagate exceptions from within the function to > the caller or the result collector. > > Fixing a bug in exception propegation discovered while writing these tests. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegionDataStore.java > 57b1e71e8a5591e74f11041c916ed5cec1a8fd29 > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceBase.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalPRDUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceLocalRRDUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceMultipleOnMemberDUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemotePRDUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceRemoteRRDUnitTest.java > PRE-CREATION > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/execute/FunctionServiceSingleOnMemberDUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/46917/diff/ > > > Testing > ------- > > > Thanks, > > Dan Smith > >
