> On Feb. 8, 2017, 7:49 p.m., Kirk Lund wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java,
> >  line 130
> > <https://reviews.apache.org/r/56425/diff/1/?file=1627169#file1627169line130>
> >
> >     I didn't notice use of "gemfire.home" before but given this is geode 
> > code, we shouldn't use "gemfire.home"
> >     
> >     I thought Anthony and Dan had worked to remove all env vars and uses of 
> > gemfire.home or geode.home from our tests and code.
> 
> Anthony Baker wrote:
>     I have no recollection of this :-)
>     
>     The only uses of `gemfire.home` are here:
>     
>     ```
>     ~/code/incubator-geode (release/1.1.0)$ git grep -i gemfire.home
>     
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java:
>     // GEODE-958: We need to set gemfire.home to tell AgentUtil where to find 
> wars in case the env
>     
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java:
>     System.out.println("Setting gemfire.home to " + installDir);
>     
> geode-assembly/src/test/java/org/apache/geode/management/internal/AgentUtilJUnitTest.java:
>     System.setProperty("gemfire.home", installDir.toString());
>     
> geode-core/src/main/java/org/apache/geode/management/internal/AgentUtil.java: 
>      logger.info("Reading gemfire.home System Property -> {}", geodeHome);
>     
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java:
>     // Setting gemfire.home to this value allows locators started by the rule 
> to run Pulse
>     
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java:
>     System.out.println("Setting gemfire.home to " + geodeInstallDir);
>     
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java:
>       System.setProperty("gemfire.home", geodeInstallDir);
>     ```
> 
> Dan Smith wrote:
>     We did move all of the code that looked for files in 
> System.getProperty(JTESTS) look up resources from the classpath instead. See 
> TestUtils.getResourcePath. I don't think we did anything with gemfire.home.

Yes, it was predominantly JTESTS. The old Ant build (in tests.xml) set all of 
the following system properties for tests to use and these are now cleaned up 
and not used in Geode. I believe most of these converted to finding resources 
from the classpath:

        <sysproperty key="GEMFIRE" value="${product.dir}"/>
        <sysproperty key="INDEXQUERYXMLWITHERRORFILE" 
value="${tests.lib.dir}/cachequeryindexwitherror.xml"/>
        <sysproperty key="INDEXQUERYXMLFILE" 
value="${tests.lib.dir}/cachequeryindex.xml"/>
        <sysproperty key="QUERYXMLFILE" 
value="${tests.lib.dir}/cachequery.xml"/>
        <sysproperty key="JTAXMLFILE" value="${tests.lib.dir}/cachejta.xml"/>
        <sysproperty key="JTESTS" value="${tests.classes.dir}"/>
        <sysproperty key="EXTRA_JTESTS" value="@{extra.jtests.dir}"/>
        <sysproperty key="TESTDIR" value="${tests.src.dir}"/>
        <sysproperty key="DistributionManager.VERBOSE" value="${dmVerbose}"/>
        <!-- uncomment the following line if you junit is running out of mem 
due to verbose logging -->
        <!-- <sysproperty key="gemfire.log-file" value="myjunit.log"/> -->
        <!-- <sysproperty key="disk.TRACE_WRITES" value="true"/> -->
        <!-- <sysproperty key="disk.TRACE_RECOVERY" value="true"/> -->
        <sysproperty key="gemfire.DEFAULT_MAX_OPLOG_SIZE" value="10"/>
        <sysproperty key="gemfire.disallowMcastDefaults" value="true"/>
        <sysproperty key="gemfire.debug" value="${gemfire.debug}"/>
        <sysproperty key="sqlfabric.debug.true" 
value="${sqlfabric.debug.true}"/>
        <sysproperty key="java.library.path" 
value="${hiddenlib.dir}${path.separator}${java.library.path}"/>
        <!-- allow running in background that gets stuck due to jline usage of 
ij -->
        <sysproperty key="jline.terminal" value="jline.UnsupportedTerminal"/>

GEMFIRE=${product.dir} was comparable to GEODE_HOME and gemfire.home

I'd like to see us remove usage of gemfire.home and only use GEODE_HOME. Also 
any test classes (rules or tests) that use GEODE_HOME should move to 
geode-assembly.

Beyond that I'd like to discuss introducing automated acceptance testing using 
JBehave+Docker and we can probably borrow some ideas from dunit for that. Maybe 
this would lead to a geode-acceptance module (or not). The JBehave "given" 
components look a lot like JUnit rules.


- Kirk


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


On Feb. 8, 2017, 3:03 a.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56425/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 3:03 a.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-2272: do not use a new method to start locator with pulse
> 
> 
> Diffs
> -----
> 
>   
> geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java
>  b5472909eec8d5ca124e7bfd5c6cb71864d9bbee 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
>  1f0cd9e720e732c2ce06515c16601e1df173ff4f 
> 
> Diff: https://reviews.apache.org/r/56425/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Reply via email to