----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53274/#review154300 -----------------------------------------------------------
Use of the Rule should be cleaned up so everything is contained inside the Rule. Otherwise there's no value in trying to package this up as a Rule. geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java (line 65) <https://reviews.apache.org/r/53274/#comment223785> This Rule isn't being used as a Rule so I'm confused? This would be correct: @Rule public GfshShellConnectionRule gfshConnector = new GfshShellConnectionRule(); The before and after should be built into the Rule so lines #78-80 in after() wouldn't be necessary either. If this is not being used as a Rule because some tests require different scaffolding from other tests, then the tests should be moved into a different test class. geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java (line 84) <https://reviews.apache.org/r/53274/#comment223787> You should keep these as "throws Exception" -- if you're calling something that requires "throws Throwable" then we should change that class/method to "throws Exception" - Kirk Lund On Oct. 28, 2016, 9:49 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53274/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2016, 9:49 p.m.) > > > Review request for geode, Kevin Duling and Kirk Lund. > > > Repository: geode > > > Description > ------- > > GEODE-1955: fix flaky tests by properly closing the gfsh instance in > ConnectToLocatorSSLDUnitTest > > > Diffs > ----- > > > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java > 8426c6f52129472c9c0def172aeefd10f1491935 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java > d3390ba6ce2990d05516b701f82d5f176e6f4d49 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java > b2dc6fe334499456184e1d0cbc0297063d4c99ce > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > 25780e67dd3f7920e53162ef9400d2845e4c958b > > Diff: https://reviews.apache.org/r/53274/diff/ > > > Testing > ------- > > > Thanks, > > Jinmei Liao > >
