1) System Rules for JUnit 4
I really like System Rules for JUnit 4. This library has been added to our
build on GitLabs develop but not in ASF git yet
http://stefanbirkner.github.io/system-rules/. This line resets all System
Properties changes you make in your tests:
@Rule
public final RestoreSystemProperties restoreSystemProperties = new
RestoreSystemProperties();
I think you could add it to open/build.gradle in ASF git if you want to use
these Rules before our next code merge (just add one line above the junit
4.11 dependency line):
testCompile 'com.github.stefanbirkner:system-rules:1.9.0'
testCompile 'junit:junit:4.11'
Or obviously you can write an @After/@AfterClass method to reset any System
Properties you set.
2) Unit test class names
I think our gradle test target requires the tests to be named *JUnitTest
for now: GfshConfigInitFileJUnitTest, GfshInitFileJUnitTest. Or we'll have
to change the gradle config to execute all Tests without "DUnit" in the
name for "test" and "integrationTest" (currently the latter targets look
for *JUnitTest).
3) Code format
Also, 2-space soft tabs is our official standard for indentation.
Changes look good! I think we should look into creating some reusable Rules
for all that logging setup you had to write in the tests.
On Fri, May 29, 2015 at 9:01 AM, Bruce Schuchardt <bschucha...@pivotal.io>
wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34814/#review85742
> -----------------------------------------------------------
>
>
> This looks good, with the exception that I think there needs to be an
> @After in the config file unit test that resets the system properties to
> what they were before the test ran.
>
> - Bruce Schuchardt
>
>
> On May 29, 2015, 3:39 p.m., Roman Shaposhnik wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/34814/
> > -----------------------------------------------------------
> >
> > (Updated May 29, 2015, 3:39 p.m.)
> >
> >
> > Review request for geode.
> >
> >
> > Bugs: GEODE-38
> > https://issues.apache.org/jira/browse/GEODE-38
> >
> >
> > Repository: geode
> >
> >
> > Description
> > -------
> >
> > Gfsh launcher script implies an init file can be used. This was possible
> in GF 6.6, but wasn't implemented in the transition to the new Gfsh in GF
> 7.0 onwards.
> > From "gemfire-assembly/src/main/dist/bin/gfsh.bat " :
> > "
> > #
> > Copy default .gfshrc to the home directory. Uncomment if needed.
> > #
> > #if [ ! -f $HOME/.gemfire/.gfsh2rc ]; then
> > cp $GEMFIRE/defaultConfigs/.gemfire/.gfsh2rc $HOME
> > #fi
> > "
> > If this file is specified, it is currently ignored.
> >
> >
> > Diffs
> > -----
> >
> >
>
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java
> d9a396a
> >
>
> gemfire-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java
> d7dba5a
> >
>
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfigInitFileTest.java
> PRE-CREATION
> >
>
> gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshInitFileTest.java
> PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/34814/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Roman Shaposhnik
> >
> >
>
>
