Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-21 Thread Jinmei Liao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/
---

(Updated July 21, 2017, 8:05 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
* For LocatorServerStarterRule, also do not set the workingDir if not asked to, 
and only bounce the VM if the VM is started with a workingDir


Diffs
-

  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
 e494bcdd8aab49864516d61ca1eee78b50a3e7ee 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
 846a8d9ab93de73e612a85a4efa781b6cc1ac4a1 
  
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
 4ca625a436ad9d181f1fecd11afc7a0cf1157aa6 
  
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/commands/ExportLogsWithMemberGroupDUnitTest.java
 e5cca279d8a8082726fc67eab8a5cfd91ce90529 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java
 766f25ada3b446df71d840d70bb6fae797a85a00 
  
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/cli/util/MergeLogsDUnitTest.java
 9166642044055071b2cced88edfe3ae660976ef7 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
 3b105a4e412542160e763d7016970e10e8c2adcf 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java
 4b003a8b2b4b0ac1df19f851be23b3442fca975f 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
 066f8828f64b7caa9c9748683f9214868105fed8 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java
 c84a7c12f62a5ee28fec24aa82ba7da6f62a7d3d 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java
 fd8fb46f44edcea2a068ec47d2a8577c933fd319 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
 bb45b0d9acbed901fa17ef39d1bb0ca1153e0458 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
 4274d677d6d06a6f9b14dd362fef9d8fcd10f410 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
 e6b50d210dad844aca5e37fcb3cf58d7aace4b3e 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  
geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
 c4a25470bab75a2fdfcdb55126564d21ecd9403d 
  
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
 226fd6c24bb1ef818fd2de2bd010d713b59872e8 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java 
7e5ce1f1e41eff125c5d2e75f389f5ea8b1f90ed 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
 6ea2d0336f74956ff81efee94fc1ac2504383520 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/test/MemberStarterRuleTest.java
 7dada04cc63aaa2668e103524bd63d3c58310257 
  
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-21 Thread Jinmei Liao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/
---

(Updated July 21, 2017, 5:11 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Changes
---

Now only a handful of our tests will change the user.dir, for Dunit tests, this 
won't be a problem since we would bounce the VM afterwards if we did change it. 
Integration tests that still calls .withLogFile can be a problem. we can change 
it to be DUnit test if we want to.


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
* For LocatorServerStarterRule, also do not set the workingDir if not asked to, 
and only bounce the VM if the VM is started with a workingDir


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
 e494bcdd8aab49864516d61ca1eee78b50a3e7ee 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
 846a8d9ab93de73e612a85a4efa781b6cc1ac4a1 
  
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
 4ca625a436ad9d181f1fecd11afc7a0cf1157aa6 
  
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/commands/ExportLogsWithMemberGroupDUnitTest.java
 e5cca279d8a8082726fc67eab8a5cfd91ce90529 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java
 766f25ada3b446df71d840d70bb6fae797a85a00 
  
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/cli/util/MergeLogsDUnitTest.java
 9166642044055071b2cced88edfe3ae660976ef7 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java
 3b105a4e412542160e763d7016970e10e8c2adcf 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java
 4b003a8b2b4b0ac1df19f851be23b3442fca975f 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java
 066f8828f64b7caa9c9748683f9214868105fed8 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java
 c84a7c12f62a5ee28fec24aa82ba7da6f62a7d3d 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java
 fd8fb46f44edcea2a068ec47d2a8577c933fd319 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java
 bb45b0d9acbed901fa17ef39d1bb0ca1153e0458 
  
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java
 4274d677d6d06a6f9b14dd362fef9d8fcd10f410 
  
geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
 e6b50d210dad844aca5e37fcb3cf58d7aace4b3e 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  
geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
 c4a25470bab75a2fdfcdb55126564d21ecd9403d 
  
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
 226fd6c24bb1ef818fd2de2bd010d713b59872e8 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java 
7e5ce1f1e41eff125c5d2e75f389f5ea8b1f90ed 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-20 Thread Jinmei Liao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/
---

(Updated July 20, 2017, 9:51 p.m.)


Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and 
Patrick Rhomberg.


Repository: geode


Description (updated)
---

* 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
* For LocatorServerStarterRule, also do not set the workingDir if not asked to, 
and only bounce the VM if the VM is started with a workingDir


Diffs (updated)
-

  
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
 4ca625a436ad9d181f1fecd11afc7a0cf1157aa6 
  
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/commands/ExportLogsWithMemberGroupDUnitTest.java
 e5cca279d8a8082726fc67eab8a5cfd91ce90529 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java
 766f25ada3b446df71d840d70bb6fae797a85a00 
  
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/cli/util/MergeLogsDUnitTest.java
 9166642044055071b2cced88edfe3ae660976ef7 
  
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/MemberMBeanSecurityJUnitTest.java
 e6b50d210dad844aca5e37fcb3cf58d7aace4b3e 
  
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 e90bc0a69222998322e02fcfad1b6bad3c97f4d1 
  
geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java
 c4a25470bab75a2fdfcdb55126564d21ecd9403d 
  
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
 226fd6c24bb1ef818fd2de2bd010d713b59872e8 
  geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java 
7e5ce1f1e41eff125c5d2e75f389f5ea8b1f90ed 
  
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
 6ea2d0336f74956ff81efee94fc1ac2504383520 
  
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
 3176f540e5605989a125d9e3b533759ce009ed42 


Diff: https://reviews.apache.org/r/60974/diff/3/

Changes: https://reviews.apache.org/r/60974/diff/2-3/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Jared Stewart 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/#review180975
---


Ship it!




Ship It!

- Jared Stewart


On July 19, 2017, 7:37 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, 7:37 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/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/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Jinmei Liao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/
---

(Updated July 19, 2017, 7:37 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 (updated)
-

  
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/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/2/

Changes: https://reviews.apache.org/r/60974/diff/1-2/


Testing
---


Thanks,

Jinmei Liao

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Jinmei Liao 


> 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)
> > 
> >
> > 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 startLocatorVMWithLogInFile(int index, 
> > Properties properties) { 
> > startLocatorVM(index, properties, true);
> > }
> > 
> >  private MemberVM 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
> 
>

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Jared Stewart 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/#review180943
---




geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
Line 92 (original), 96 (patched)


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 startLocatorVMWithLogInFile(int index, Properties 
properties) { 
startLocatorVM(index, properties, true);
}

 private MemberVM 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)
```


- Jared Stewart


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
> 
>

Re: Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Kirk Lund 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/#review180937
---


Ship it!




+1

- Kirk Lund


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
> 
>

Review Request 60974: GEODE-3231: do not log to file by default when using the member starter rules

2017-07-19 Thread Jinmei Liao 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60974/
---

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