> On May 23, 2016, 2:59 p.m., Robert Nettleton wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java,
> >  line 2395
> > <https://reviews.apache.org/r/47726/diff/1/?file=1391619#file1391619line2395>
> >
> >     +1 for Sebastian's comment, a unit test should definitely be added here 
> > to verify this change.
> 
> Daniel Gergely wrote:
>     We were talking about this unit test thing. These are static variables, 
> so I can test if all the properties are there in the map. However it is not 
> easy to test whether the correct PropertyUpdater is used with the correct 
> component. These are not visible from outside, so a complex mocking or using 
> Reflection could made it possible, but these are messy things. So he dropped 
> the issue here.

Hi Daniel, 

It should be pretty simple to unit test this issue, by verifiying that this 
property is updated as expected, when the expected set of components are 
included in the Blueprint/Cluster Configuration.  There is no need to verify 
the static maps being modified, but there is definitely a need to make sure 
that this property is handled correctly by the Blueprint export process, so I 
still think a unit test is necessary here. 

Thanks.


- Robert


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


On May 23, 2016, 2:07 p.m., Daniel Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47726/
> -----------------------------------------------------------
> 
> (Updated May 23, 2016, 2:07 p.m.)
> 
> 
> Review request for Ambari, Balázs Bence Sári, Laszlo Puskas, Oliver Szabo, 
> Robert Nettleton, Sandor Magyari, Sumit Mohanty, and Sebastian Toader.
> 
> 
> Bugs: AMBARI-16820
>     https://issues.apache.org/jira/browse/AMBARI-16820
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> MultiHostUpdater was not assigned to the property 
> hive.llap.zk.sm.connectionString
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  9cabdbb 
> 
> Diff: https://reviews.apache.org/r/47726/diff/
> 
> 
> Testing
> -------
> 
> In progress...
> 
> 
> Thanks,
> 
> Daniel Gergely
> 
>

Reply via email to