> On May 16, 2017, 11:07 p.m., Jared Stewart wrote:
> > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/59316/diff/1/?file=1721708#file1721708line45>
> >
> >     I was a bit surprised that these `set` methods need to be called after 
> > `before`.  Perhaps this would be more clear if `getRepository()` was 
> > replaced with a method like  
> >     ```
> >     public Cluster connect() { 
> >     return repository.getCluster();
> >     }
> >     ``` 
> >     that indicates the point where the `set` methods must be called before.

this is all due to the evil of Repository.get() will return a singleton. 
PulseVerificationTest needs to test using either jmx port or locator port, 
hence the need for these methods. I reworked the rule a bit to cleanup the 
state.


> On May 16, 2017, 11:07 p.m., Jared Stewart wrote:
> > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/IClusterUpdater.java
> > Lines 36 (patched)
> > <https://reviews.apache.org/r/59316/diff/1/?file=1721717#file1721717line36>
> >
> >     What is the benefit of a default implementation that returns null?

there are bunch of MockClusterUpdator used in the old tests. I added this 
interface and provide a null implementation to avoid updating those mocks.


- Jinmei


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


On May 16, 2017, 10:48 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59316/
> -----------------------------------------------------------
> 
> (Updated May 16, 2017, 10:48 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick 
> Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * using local mbs server connection will bypass all the mbean security checks
> * do not update the mbean attribute since pulse user has no cluster:write 
> privilege at all
> * Created EmbeddedPulseRule for tests
> * simplify PulseAppListener
> * use log4j2 logging configurations
> 
> 
> Diffs
> -----
> 
>   geode-pulse/src/main/resources/log4j2.properties PRE-CREATION 
>   geode-pulse/src/main/resources/log4j2.xml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59316/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Reply via email to