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