You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/03/02 15:30:38 UTC
Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/
-----------------------------------------------------------
Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
Repository: geode
Description
-------
GEODE-2267: Enhance server/locator startup rules to include workingDir
* This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
* There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
Diffs
-----
geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
Diff: https://reviews.apache.org/r/57242/diff/1/
Testing
-------
precheckin successful
Thanks,
Jinmei Liao
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
> On March 2, 2017, 6:44 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653592#file1653592line166>
> >
> > I don't think that we ought to manually delete the workingDir. If someone is using a temporaryFolder (which I think is the normal use case), then the deletion will happen automatically. If they aren't using a temporary folder, then deleting the workingDir seems dangerous.
this is there so that when we are writing test using this rule alone, we don't need to have the have a temperoaryFolder rule along side with it. The rule itself handles creating the temeroray folder and delete it afterwards. The deletion happens in the after() block, so it should be pretty safe.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167712
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
> On March 2, 2017, 6:44 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653592#file1653592line166>
> >
> > I don't think that we ought to manually delete the workingDir. If someone is using a temporaryFolder (which I think is the normal use case), then the deletion will happen automatically. If they aren't using a temporary folder, then deleting the workingDir seems dangerous.
>
> Jinmei Liao wrote:
> this is there so that when we are writing test using this rule alone, we don't need to have the have a temperoaryFolder rule along side with it. The rule itself handles creating the temeroray folder and delete it afterwards. The deletion happens in the after() block, so it should be pretty safe.
>
> Jared Stewart wrote:
> As a user of this rule, the following would seem like a reasonable thing to do:
>
> ```
> private final File workingDir = new File(".");
>
> @Rule
> ServerStarterRule serverStarterRule = new ServerStarterRule(workingDir);
> ```
>
>
> Yet it would end up deleting part of the user's geode checkout. Perhaps we should have separate constructors or factory methods for using this rule alone vs. using it through LocatorServerStarterRule. That way, when used alone, it can create its own temporary folder so that a user doesn't have to worry about it.
The reason behind this refactoring is that the normal use case is that user don't need to use a temporaryFolder with this rule. the rule handles the create/deleting the temp folder for you, like this:
@Rule
public ServerStarterRule serverStarterRule = new ServerStarterRule();
@Test
public void test(){
Server server = serverStarterRule.startServer();
File workingDir = server.getWorkingDir();
// verfify the working dir
}
And the rule will handle deleting the working dir created afterwards. Maybe you are suggesting that if user passes in a workingDir, then you are responsible of deleting it, the rule won't?
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167712
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jared Stewart <js...@pivotal.io>.
> On March 2, 2017, 6:44 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
> > Lines 154 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653592#file1653592line166>
> >
> > I don't think that we ought to manually delete the workingDir. If someone is using a temporaryFolder (which I think is the normal use case), then the deletion will happen automatically. If they aren't using a temporary folder, then deleting the workingDir seems dangerous.
>
> Jinmei Liao wrote:
> this is there so that when we are writing test using this rule alone, we don't need to have the have a temperoaryFolder rule along side with it. The rule itself handles creating the temeroray folder and delete it afterwards. The deletion happens in the after() block, so it should be pretty safe.
>
> Jared Stewart wrote:
> As a user of this rule, the following would seem like a reasonable thing to do:
>
> ```
> private final File workingDir = new File(".");
>
> @Rule
> ServerStarterRule serverStarterRule = new ServerStarterRule(workingDir);
> ```
>
>
> Yet it would end up deleting part of the user's geode checkout. Perhaps we should have separate constructors or factory methods for using this rule alone vs. using it through LocatorServerStarterRule. That way, when used alone, it can create its own temporary folder so that a user doesn't have to worry about it.
>
> Jinmei Liao wrote:
> The reason behind this refactoring is that the normal use case is that user don't need to use a temporaryFolder with this rule. the rule handles the create/deleting the temp folder for you, like this:
>
> @Rule
> public ServerStarterRule serverStarterRule = new ServerStarterRule();
>
> @Test
> public void test(){
> Server server = serverStarterRule.startServer();
> File workingDir = server.getWorkingDir();
> // verfify the working dir
> }
>
> And the rule will handle deleting the working dir created afterwards. Maybe you are suggesting that if user passes in a workingDir, then you are responsible of deleting it, the rule won't?
Yes, precisely. I think the creator of the directory should be the one responsible for deleting it.
- Jared
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167712
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167712
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
Lines 154 (patched)
<https://reviews.apache.org/r/57242/#comment239617>
I don't think that we ought to manually delete the workingDir. If someone is using a temporaryFolder (which I think is the normal use case), then the deletion will happen automatically. If they aren't using a temporary folder, then deleting the workingDir seems dangerous.
- Jared Stewart
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Kirk Lund <ki...@gmail.com>.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
> > Lines 67 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653589#file1653589line67>
> >
> > Wouldn't it be better to just Chain TemporaryFolder or DistributedTemporaryFolder with this Rule? Or maybe pass TemporaryFolder.getRoot() into the constructor on this Rule, and then let TemporaryFolder handle the before/after for itself. Now we'll have two rules that provide temporary directories.
>
> Jinmei Liao wrote:
> We were doing that before, but rules inside rules are not behaving correctly. We need to manually call the before and after the rules we used. Will look into this more.
>
> Jared Stewart wrote:
> This seems like the intended use case of RuleChain:
>
> ```
> TemporaryFolder temporaryFolder = new TemporaryFolder();
> LocatorServerStarterRule locatorServerStarterRule = new LocatorServerStarterRule(temporaryFolder);
>
> @Rule
> RuleChain ruleChain = RuleChain.outerRule(temporaryFolder).aroundRule(locatorServerStarterRule);
> ```
>
> Still, the above requires a lot of boilerplate, and I think makes the rule harder to use. Just throwing out an idea.
You can also internally chain embedded rules. Not sure why that was causing problems before, but org.apache.geode.test.junit.rules.ExpectedTimeoutRule contains and delegates to an ExpectedException rule. The implementation has to contain something like one of the following...
```java
@Override
public Statement apply(final Statement base, final Description description) {
Statement next = this.delegate.apply(base, description); <-- inserts the delegate
return new ExpectedTimeoutStatement(next); <-- wraps the delegate which wraps the next rule (or test if last rule)
}
```
Or...
```java
private Statement statement(final Statement base) {
return new Statement() {
@Override
public void evaluate() throws Throwable {
before();
try {
base.evaluate(); <-- invokes the next rule or the test if this is the last rule
} finally {
after();
}
}
};
}
```
- Kirk
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167694
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
> > Lines 66 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653572#file1653572line66>
> >
> > I think it would best if you can keep this self-contained as a ClassRule instead of invoking a rule method in BeforeClass:
> > ```java
> > @ClassRule
> > public static ServerStarterRule serverStarter = new ServerStarterRule().startServer(properties);
> > ```
> > Or (this matches the new pattern than JUnit folks are pushing for complex rules):
> > ```java
> > @ClassRule
> > public static ServerStarterRule serverStarter = ServerStarterRule.builder().startServer(properties).build();
> > ```
> > And then change startServer to return this (ServerStarterRule).
Good idea. Will enhance this more in the next round.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java
> > Line 80 (original), 80 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653576#file1653576line80>
> >
> > I would override startLocatorVM to have a flavor that doesn't require new Properties.
> > ```java
> > locator = lsRule.startLocatorVM(0);
> > ```
good idea. It was added at one point.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
> > Line 399 (original), 399 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653578#file1653578line399>
> >
> > I know I mentioned it before, but I really don't like seeing a Rule used as not-a-Rule.
> >
> > It would be much cleaner to create a ServerStarter class. Then use ServerStarter directly where you don't need a Rule and use ServerStarterRule only where you need a Rule (ie, when JUnit lifecycle is responsible for invoking before and after instead of invoking directly like this).
We usually wouldn't need to do this. This is an old tests that is not using LocatorServerStarterRule, and I need a quick a way to start a server in the vm. I will add a note in the test saying this is not a recommended usage.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java
> > Lines 59 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653581#file1653581line64>
> >
> > This could all be moved into a builder on the Rule. See org.junit.rules.Timeout and org.apache.geode.management.ManagementTestRule:
> > ```java
> > @ClassRule
> > public static ServerStarterRule serverRule = ServerStarterRule.builder()
> > .setProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName())
> > .setProperty(JMX_MANAGER_PORT, jmxManagerPort + "")
> > .setProperty(JMX_MANAGER_PORT, jmxManagerPort + "")
> > .startServer()
> > .build();
> > ```
> > This is the pattern/style recommended for complex rules (it's not my invention).
Good idea. Will consider this in the next round of refactoring.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
> > Lines 67 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653589#file1653589line67>
> >
> > Wouldn't it be better to just Chain TemporaryFolder or DistributedTemporaryFolder with this Rule? Or maybe pass TemporaryFolder.getRoot() into the constructor on this Rule, and then let TemporaryFolder handle the before/after for itself. Now we'll have two rules that provide temporary directories.
We were doing that before, but rules inside rules are not behaving correctly. We need to manually call the before and after the rules we used. Will look into this more.
> On March 2, 2017, 6:39 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653592#file1653592line76>
> >
> > I've this behave differently on different operating systems and misbehave in general. It creates the illusion to the next developer that this system property is mutable, and then it can be nightmare to debug when they use a JDK API that theoretically uses "user.dir" but still ends up using the original "user.dir" -- I would do everything we can to eliminate usage of "user.dir" or back it into a corner (one single line of code in all of Geode).
> >
> > We can alter any Geode code that relies on relative paths to have absolute paths always passed into them. Then this problem goes away.
> >
> > Or at the very least it pushes getProperty("user.dir") into a single method that finds the root directory for the member. Then you can override this or mock it to have it return temporaryFolder.getRoot() instead of getProperty("user.dir").
> > ```java
> > public class WorkingDirectory {
> >
> > // override or mock me
> > public File getWorkingDirectory() {
> > return System.getProperty("user.dir");
> > }
> > ```
> > Or:
> > ```java
> > public class WorkingDirectory {
> >
> > private File dir;
> >
> > public WorkingDirectory(String path) {
> > this.dir = new File(path);
> > }
> >
> > public File getWorkingDirectory() {
> > return this.dir;
> > }
> > ```
> > Then alter our Geode code to use the above. Immutable system properties and static code is a nightmare in tests.
Agree, right now, there is no fixed way how how Geode code is handling relative paths. It was a nightmare trying to figure this out, and we do need a consistent way of doing that both in our test code and in our production code.
This is only a refactoring of the old LocatorServerStartupRule where it was doing the same thing before (setting user.dir in each of the VMs when starting up locator/server in the VM). I am simply moving it to here so that when using ServerStartRule alone, we have a notion of where its workingdir is for testing purpose. We should file a separate JIRA ticket to do the way you are suggesting.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167694
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167694
-----------------------------------------------------------
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
Lines 66 (patched)
<https://reviews.apache.org/r/57242/#comment239601>
I think it would best if you can keep this self-contained as a ClassRule instead of invoking a rule method in BeforeClass:
```java
@ClassRule
public static ServerStarterRule serverStarter = new ServerStarterRule().startServer(properties);
```
Or (this matches the new pattern than JUnit folks are pushing for complex rules):
```java
@ClassRule
public static ServerStarterRule serverStarter = ServerStarterRule.builder().startServer(properties).build();
```
And then change startServer to return this (ServerStarterRule).
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java
Line 80 (original), 80 (patched)
<https://reviews.apache.org/r/57242/#comment239602>
I would override startLocatorVM to have a flavor that doesn't require new Properties.
```java
locator = lsRule.startLocatorVM(0);
```
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java
Line 399 (original), 399 (patched)
<https://reviews.apache.org/r/57242/#comment239603>
I know I mentioned it before, but I really don't like seeing a Rule used as not-a-Rule.
It would be much cleaner to create a ServerStarter class. Then use ServerStarter directly where you don't need a Rule and use ServerStarterRule only where you need a Rule (ie, when JUnit lifecycle is responsible for invoking before and after instead of invoking directly like this).
geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java
Lines 59 (patched)
<https://reviews.apache.org/r/57242/#comment239605>
This could all be moved into a builder on the Rule. See org.junit.rules.Timeout and org.apache.geode.management.ManagementTestRule:
```java
@ClassRule
public static ServerStarterRule serverRule = ServerStarterRule.builder()
.setProperty(SECURITY_MANAGER, SimpleTestSecurityManager.class.getName())
.setProperty(JMX_MANAGER_PORT, jmxManagerPort + "")
.setProperty(JMX_MANAGER_PORT, jmxManagerPort + "")
.startServer()
.build();
```
This is the pattern/style recommended for complex rules (it's not my invention).
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java
Lines 67 (patched)
<https://reviews.apache.org/r/57242/#comment239607>
Wouldn't it be better to just Chain TemporaryFolder or DistributedTemporaryFolder with this Rule? Or maybe pass TemporaryFolder.getRoot() into the constructor on this Rule, and then let TemporaryFolder handle the before/after for itself. Now we'll have two rules that provide temporary directories.
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
Lines 76 (patched)
<https://reviews.apache.org/r/57242/#comment239610>
I've this behave differently on different operating systems and misbehave in general. It creates the illusion to the next developer that this system property is mutable, and then it can be nightmare to debug when they use a JDK API that theoretically uses "user.dir" but still ends up using the original "user.dir" -- I would do everything we can to eliminate usage of "user.dir" or back it into a corner (one single line of code in all of Geode).
We can alter any Geode code that relies on relative paths to have absolute paths always passed into them. Then this problem goes away.
Or at the very least it pushes getProperty("user.dir") into a single method that finds the root directory for the member. Then you can override this or mock it to have it return temporaryFolder.getRoot() instead of getProperty("user.dir").
```java
public class WorkingDirectory {
// override or mock me
public File getWorkingDirectory() {
return System.getProperty("user.dir");
}
```
Or:
```java
public class WorkingDirectory {
private File dir;
public WorkingDirectory(String path) {
this.dir = new File(path);
}
public File getWorkingDirectory() {
return this.dir;
}
```
Then alter our Geode code to use the above. Immutable system properties and static code is a nightmare in tests.
- Kirk Lund
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Kevin Duling <kd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167696
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Duling
On March 2, 2017, 7:30 a.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 7:30 a.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/
-----------------------------------------------------------
(Updated March 7, 2017, 6:24 p.m.)
Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
Repository: geode
Description
-------
GEODE-2267: Enhance server/locator startup rules to include workingDir
* This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
* There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
New round of refactoring on top of the old one:
* be able to return the rule itself so that we can start the server/locator at rule declaration time.
* rearrange the class structure
* do not delete the workingDir if the rule is created with a workingDir (then it's up for the caller to delete it)
I will need to get the batch in after our export log merge though.
Diffs
-----
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java 75a3c2c0996972cab26dc6dad79f675ddfb8f6b3
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 933f7b2ad73b76def7fb1029d4f424f77d1a4211
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java fa98ce69559ed5bbdd1eb320734090c3de007106
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb
geode-core/src/test/java/org/apache/geode/management/internal/cli/NetstatDUnitTest.java c6248e052ea8d8579c4f5b2b4de9ec74dca30e72
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java 4ac98864eed9330fce519e4a7054cb8ab6201cae
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java 15e6ea6ec15759800f07e05e8a2bbc6b2d586754
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java e134c42ed88ee7f61e76dcb4503b7ffc13bd2276
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java 72daf0d07feb25a23be84b4a2aae1c1a7668e64c
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java c0d22bfc697e886cbdb074d9ad5b3241dab93e03
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java 4d67fb0fe4d54299b4b2a523aee0d9126a1e3836
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java 30e1df85da4b6040594ef81bbf9ff5b0dc6f2c31
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 646819559f608e8262b90e6774f62b2b15b56a82
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java d1750c3a5efd636c0f9f47fabe670265bf46f072
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java b6d108016f44e3bbafd6c3975d9827a7ac479706
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java 5d713f6bf91fcd74e77ba6ed8c1040c605a954ce
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java 12c2da3d000669ea3b28d52bd80d96a17adfa15e
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java 092e82b82a8438a3be3394fd997ec727cda5b782
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java f5cfff613300b758b20f3c2acfb875cfa4804d79
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java 4729be3f4d9b51168422784a57ac1ec76e018e83
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java b1004b9cee8c2f2aae96664966aa0e6c7a565fdc
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 39c13d000db80bef37563729bc17ae4bcb566153
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java 84c660cfe0ab9b7c6ba0bfc08b034ccaf17697f0
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 5f46da21f380a67a8e7f4855da1dfbb3118057ba
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 83093c4c8963b64020d7ed8f573e904fccd32e2e
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java df3757950c2fd28521958416d763cb1e41ae6456
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java 1a344db6be8003fe21bb505362e77a1864e88264
Diff: https://reviews.apache.org/r/57242/diff/2/
Testing (updated)
-------
rebased this last changeset to the current develop, precheckin successful.
Thanks,
Jinmei Liao
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/
-----------------------------------------------------------
(Updated March 3, 2017, 12:03 a.m.)
Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
Repository: geode
Description (updated)
-------
GEODE-2267: Enhance server/locator startup rules to include workingDir
* This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
* There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
New round of refactoring on top of the old one:
* be able to return the rule itself so that we can start the server/locator at rule declaration time.
* rearrange the class structure
* do not delete the workingDir if the rule is created with a workingDir (then it's up for the caller to delete it)
I will need to get the batch in after our export log merge though.
Diffs
-----
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java 75a3c2c0996972cab26dc6dad79f675ddfb8f6b3
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 933f7b2ad73b76def7fb1029d4f424f77d1a4211
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java fa98ce69559ed5bbdd1eb320734090c3de007106
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb
geode-core/src/test/java/org/apache/geode/management/internal/cli/NetstatDUnitTest.java c6248e052ea8d8579c4f5b2b4de9ec74dca30e72
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java 4ac98864eed9330fce519e4a7054cb8ab6201cae
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java 15e6ea6ec15759800f07e05e8a2bbc6b2d586754
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java e134c42ed88ee7f61e76dcb4503b7ffc13bd2276
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java 72daf0d07feb25a23be84b4a2aae1c1a7668e64c
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java c0d22bfc697e886cbdb074d9ad5b3241dab93e03
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java 4d67fb0fe4d54299b4b2a523aee0d9126a1e3836
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java 30e1df85da4b6040594ef81bbf9ff5b0dc6f2c31
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 646819559f608e8262b90e6774f62b2b15b56a82
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java d1750c3a5efd636c0f9f47fabe670265bf46f072
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java b6d108016f44e3bbafd6c3975d9827a7ac479706
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java 5d713f6bf91fcd74e77ba6ed8c1040c605a954ce
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java 12c2da3d000669ea3b28d52bd80d96a17adfa15e
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java 092e82b82a8438a3be3394fd997ec727cda5b782
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java f5cfff613300b758b20f3c2acfb875cfa4804d79
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java 4729be3f4d9b51168422784a57ac1ec76e018e83
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java b1004b9cee8c2f2aae96664966aa0e6c7a565fdc
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 39c13d000db80bef37563729bc17ae4bcb566153
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java 84c660cfe0ab9b7c6ba0bfc08b034ccaf17697f0
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 5f46da21f380a67a8e7f4855da1dfbb3118057ba
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 83093c4c8963b64020d7ed8f573e904fccd32e2e
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java df3757950c2fd28521958416d763cb1e41ae6456
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java 1a344db6be8003fe21bb505362e77a1864e88264
Diff: https://reviews.apache.org/r/57242/diff/2/
Testing
-------
precheckin successful
Thanks,
Jinmei Liao
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/
-----------------------------------------------------------
(Updated March 3, 2017, 12:01 a.m.)
Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
Changes
-------
precheckin running
Repository: geode
Description
-------
GEODE-2267: Enhance server/locator startup rules to include workingDir
* This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
* There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
Diffs (updated)
-----
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java 75a3c2c0996972cab26dc6dad79f675ddfb8f6b3
geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 933f7b2ad73b76def7fb1029d4f424f77d1a4211
geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java fa98ce69559ed5bbdd1eb320734090c3de007106
geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 83a367eb88aec85984691e651e5de0f8b479c7cb
geode-core/src/test/java/org/apache/geode/management/internal/cli/NetstatDUnitTest.java c6248e052ea8d8579c4f5b2b4de9ec74dca30e72
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java 4ac98864eed9330fce519e4a7054cb8ab6201cae
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java 15e6ea6ec15759800f07e05e8a2bbc6b2d586754
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java e134c42ed88ee7f61e76dcb4503b7ffc13bd2276
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java 72daf0d07feb25a23be84b4a2aae1c1a7668e64c
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java c0d22bfc697e886cbdb074d9ad5b3241dab93e03
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigWithSecurityDUnitTest.java 4d67fb0fe4d54299b4b2a523aee0d9126a1e3836
geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java 30e1df85da4b6040594ef81bbf9ff5b0dc6f2c31
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 646819559f608e8262b90e6774f62b2b15b56a82
geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java d1750c3a5efd636c0f9f47fabe670265bf46f072
geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java b6d108016f44e3bbafd6c3975d9827a7ac479706
geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java 5d713f6bf91fcd74e77ba6ed8c1040c605a954ce
geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java 12c2da3d000669ea3b28d52bd80d96a17adfa15e
geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java 092e82b82a8438a3be3394fd997ec727cda5b782
geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java f5cfff613300b758b20f3c2acfb875cfa4804d79
geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java 4729be3f4d9b51168422784a57ac1ec76e018e83
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java b1004b9cee8c2f2aae96664966aa0e6c7a565fdc
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 39c13d000db80bef37563729bc17ae4bcb566153
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java 84c660cfe0ab9b7c6ba0bfc08b034ccaf17697f0
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 5f46da21f380a67a8e7f4855da1dfbb3118057ba
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberVM.java PRE-CREATION
geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 83093c4c8963b64020d7ed8f573e904fccd32e2e
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java df3757950c2fd28521958416d763cb1e41ae6456
geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java 1a344db6be8003fe21bb505362e77a1864e88264
Diff: https://reviews.apache.org/r/57242/diff/2/
Changes: https://reviews.apache.org/r/57242/diff/1-2/
Testing
-------
precheckin successful
Thanks,
Jinmei Liao
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Jinmei Liao <ji...@pivotal.io>.
> On March 2, 2017, 6:31 p.m., Ken Howe wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Line 140 (original), 114 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653588#file1653588line144>
> >
> > Why is arg changed from 0 to -1? See comment on ServerStartRule
"0" has some confusing meanings. In our product code, sometimes if we set a port to 0, it means "use random port". Here I use "-1" to emphasize the fact that I don't want this server to connect to any locator, it's a standalone server.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167703
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Ken Howe <kh...@pivotal.io>.
> On March 2, 2017, 6:31 p.m., Ken Howe wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Line 140 (original), 114 (patched)
> > <https://reviews.apache.org/r/57242/diff/1/?file=1653588#file1653588line144>
> >
> > Why is arg changed from 0 to -1? See comment on ServerStartRule
>
> Jinmei Liao wrote:
> "0" has some confusing meanings. In our product code, sometimes if we set a port to 0, it means "use random port". Here I use "-1" to emphasize the fact that I don't want this server to connect to any locator, it's a standalone server.
I agree with the intent, but without code to back up the use of -1, it's just a magic number that that we now need to infer the intent thru code inspection.
Maybe enforce the standalone server usage by providing a startServerVM variant using a boolean argument, e.g. standaloneServer = true
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167703
-----------------------------------------------------------
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 57242: GEODE-2267: Enhance server/locator startup
rules to include workingDir
Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57242/#review167703
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
Line 140 (original), 114 (patched)
<https://reviews.apache.org/r/57242/#comment239613>
Why is arg changed from 0 to -1? See comment on ServerStartRule
geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java
Line 79 (original), 92 (patched)
<https://reviews.apache.org/r/57242/#comment239611>
Why use locatorPort from 0 to -1? The test in startServer(Properties properties, int locatorPort, boolean pdxPersistent) is for >0
- Ken Howe
On March 2, 2017, 3:30 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57242/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 3:30 p.m.)
>
>
> Review request for geode, Jared Stewart, Kevin Duling, Ken Howe, and Kirk Lund.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-2267: Enhance server/locator startup rules to include workingDir
>
> * This batch is mostly test code change, I am enhancing the ServerStartupRules and LocatorStartupRules to have a notion of workingDir itself.
> * There are only 2 product code change: one is the assembly's build.gradle to correctly set the gemfire.home/geode.home, another one is to use an absolute path to check for File's exsistance. Without that, the tests always fail in CI pipeline.
>
>
> Diffs
> -----
>
> geode-assembly/build.gradle 1c95927f19888b15e771b3cbbc92f120787868c4
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java 3a29e8a435f5d5362810f5e05ea92a1260e3fbfc
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java f0b6bb327f0424f2e4afa2e057d8e38bd0ef1b39
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java 160f6349d94646b12e3114da14e5bff569eed5a9
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java 10ce515df8c3630cbf01fc314be454d3e2adfe2c
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java c7d0e7341fcf13bf34a597405d2b42b608004215
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java b5472909eec8d5ca124e7bfd5c6cb71864d9bbee
> geode-core/src/main/java/org/apache/geode/distributed/DistributedSystem.java 20c948f8f16240aef5c8eae313164693125a99ca
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java 05f2c0cb2115ee574666ed3da943c26e8c38c9f2
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java 3183aafd9907ecb15714c2038f2b8c3af31e6e06
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java ea72cfab26f828af9a09f08755a9a7f9ed121654
> geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java e89bd62b59b8218c30c8f724b4febf2b3ae7721b
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java 0980c18de1b504f545355a0f7d650f46c92cc670
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java 2d32905d45496fe69e5dc189cd94b82ea163c3b5
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java 2ae61402d330ab6426c0bc0e0193771ce5dc9c70
> geode-core/src/test/java/org/apache/geode/security/PeerAuthenticatorDUnitTest.java 52afab40990ab3a18fed04fcb001ec9dbe7d045e
> geode-core/src/test/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java e87a285d088c923326aa82ee406f543ffc5e0593
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Locator.java fe26e8364798689c1ece487554d99085f3f26f23
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java 68447201875ef31a73cfb888d346292e3f084ae8
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java c3e493e1c176974ed757c71d5c195ff5964dbb57
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Member.java 7cc1eea2e26b176b570111bccdb8914e19528ecf
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/Server.java 4aa2c69389dce4650cf476f4e8decebddfc36736
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java b5ddee666c52fe39deb015c6e2aa57e5b21e4f20
> geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java 1a19e5e5ca90c248655eb40e68aca3c7ed858770
>
>
> Diff: https://reviews.apache.org/r/57242/diff/1/
>
>
> Testing
> -------
>
> precheckin successful
>
>
> Thanks,
>
> Jinmei Liao
>
>