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