> On June 29, 2017, 5:34 p.m., Kirk Lund wrote: > > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java > > Line 63 (original), 66 (patched) > > <https://reviews.apache.org/r/60347/diff/1/?file=1757905#file1757905line79> > > > > Looks like this test has some problems. The Rules are not marked with > > @Rule, so the test would be responsible for invoking before and after on > > each Rule. I've said this before: a Rule that's not used as a Rule is > > "astonishing" or "surprising" -- > > https://en.wikipedia.org/wiki/Principle_of_least_astonishment > > Jared Stewart wrote: > The Rules are being rules in this case, they're just being called through > a `RuleChain` since we need to ensure order. Do you have any suggestions for > how to make that more apparent?
Oops, I missed the RuleChain because I didn't open nearby lines. Sorry about that. - Kirk ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60347/#review179282 ----------------------------------------------------------- On June 21, 2017, 10:48 p.m., Jared Stewart wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60347/ > ----------------------------------------------------------- > > (Updated June 21, 2017, 10:48 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-3097: GFSH works over HTTP with SSL > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java > 2da95a7c3f305aab4e615d4be7b14c19b9b31dbc > > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java > 844e0322eecdc93c4d46c546f4df2d278c5f15cd > > > Diff: https://reviews.apache.org/r/60347/diff/1/ > > > Testing > ------- > > Precheckin running > > > Thanks, > > Jared Stewart > >