> On July 19, 2017, 4:49 p.m., Jared Stewart wrote: > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > > Line 92 (original), 96 (patched) > > <https://reviews.apache.org/r/60974/diff/1/?file=1779518#file1779518line96> > > > > I think that having two differently-named public methods (rather than > > one method with a boolean flag) might express the intent of the tests > > calling this method more clearly. > > > > Basically I'm thinking something like: > > ``` > > public MemberVM<Locator> startLocatorVMWithLogInFile(int index, > > Properties properties) { > > startLocatorVM(index, properties, true); > > } > > > > private MemberVM<Locator> startLocatorVM(int index, Properties > > properties, boolean withLogInFile) > > ``` > > > > The three-arg variant could still be around under the hood as a private > > method, but the tests would read like: > > ``` > > startLocatorVMWithLogInFile(0, props) > > ``` > > > > rather than > > ``` > > startLocatorVM(0,props,true) > > ```
good idea, I will do that. I hate adding more flag in the parameter list. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60974/#review180943 ----------------------------------------------------------- On July 19, 2017, 4:06 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60974/ > ----------------------------------------------------------- > > (Updated July 19, 2017, 4:06 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > * do not log to file by default, only when withLogInFile is called > * do not create a workingDir and sets the user.dir by default, only when > withWorkingDir is called > > > Diffs > ----- > > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java > 045e13edc798f2bd3460dc17bbfeec28ac5022e4 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java > 0b012c9569236e2d368d88b14bf506ac9bd5d8be > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsOnServerManagerDUnit.java > a78b26fb560c17c96b794ecedc09e10c87766ef0 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java > 33439e6063150b87938f21f13e647d3b0ecc4896 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java > d98641849eed458b4c830e1fa198f4d986b0c4ce > > geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java > 510a4e7fef1aff5a1aa8f00f569cd6422b7816ab > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java > a8b498ddbecb9d8733ed4f98c1c80cb18da5c37e > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java > 066f8828f64b7caa9c9748683f9214868105fed8 > > geode-core/src/test/java/org/apache/geode/management/internal/security/MultiUserDUnitTest.java > e3fe173e7d00686df5bf3b9e3212f3dca4c161c7 > > geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java > e90bc0a69222998322e02fcfad1b6bad3c97f4d1 > > geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java > b977b50e28b8d518fd3d9da6c03b55f8b7cbff34 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > 62ae2e223f019c78000ff7492510c24aba9c930c > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > 5b4f2c14545847d9a3fcfc501fdd9ff1e19771ba > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/test/MemberStarterRuleTest.java > 7dada04cc63aaa2668e103524bd63d3c58310257 > > geode-web/src/test/java/org/apache/geode/management/internal/security/LogNoPasswordTest.java > 688af782b8a7e6d12aca38694f9df05ac8a1bef7 > > > Diff: https://reviews.apache.org/r/60974/diff/1/ > > > Testing > ------- > > > Thanks, > > Jinmei Liao > >