> 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.
> 
> Robert Nettleton wrote:
>     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.

Here's an example of a unit test that verifies that the property updaters are 
handling Blueprint exports correctly:

org.apache.ambari.server.controller.internal.BlueprintConfigurationProcessorTest#testYarnConfigExported

There are many more examples of this in this test case as well.

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