> On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > > Line 107 (original), 107 (patched) > > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line109> > > > > I like this use of `IntStream`!
+1 Much clearer to read > On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > > Lines 232 (patched) > > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line243> > > > > Do you think there is any chance someone will start both a server and a > > client in the same VM? (To deal with that I think we would just remove the > > `else{` and always try to disconnect the DS, even if `member` was not > > `null`) > > Jinmei Liao wrote: > the member.stopMember() eventually calls MemberStarerRule.stopMember() > which will call disconnectDS already. Is it still necessary to this? > > Jared Stewart wrote: > Hm, maybe I'm missing something. All I see in > MemberStarterRule.stopMember() is ` abstract void stopMember();`, and it > doesn't look like either of the concrete implementations (in > LocatorStarterRule and ServerStarterRule) explicitly disconnect the DS. > Perhaps `InternalLocator.stop()` and `CacheServer.stop()` already disconnect > the DS? If that's the case I don't think it should be necessary for us to > disconnect it again. I agree with Jinmei, looks to me that MemberStarterRule.stopMember() eventually calls MemberStarterRule.after(), which will then disconnectDSIfAny - Ken ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62179/#review184984 ----------------------------------------------------------- On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62179/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2017, 3:51 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > Repository: geode > > > Description > ------- > > Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the > ClientVM > > > Diffs > ----- > > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java > 1a5fc3485e159ca311247e617d5bec2b37c6ee10 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java > 3e9227e99ad57624b024498d0ff5e807c8853221 > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java > 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > f0385e21f708d9a085e7129d82fb3101e2fb8322 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > a832a2590527100afc05fb9de2e332a263d52c19 > > geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java > b35270e2d97930cee68d8c54221a04c20dfb96de > > geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java > c301d587c04651a03170e3da6451ebf2acf063c0 > > > Diff: https://reviews.apache.org/r/62179/diff/2/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
