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



common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
<https://reviews.apache.org/r/33867/#comment133634>

    If the purpose of the catch block is to validate the error message then it 
can be done in better manner by leveraging expectedExceptionMessageRegExp 
parameter. You can pass the message string or a regular expression.
    
    
http://testng.org/javadoc/org/testng/annotations/Test.html#expectedExceptionsMessageRegExp()
    
    Doing exact string match can cause unwanted coupling of the error messages 
in tests and source.



common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
<https://reviews.apache.org/r/33867/#comment133635>

    Seems like cleanupClusterLocations is a good candidate for @AfterMethod.



common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
<https://reviews.apache.org/r/33867/#comment133631>

    Since only one argument is being passed, any particular reason to use 
isNoneEmpty here instead of isNotEmpty?



common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
<https://reviews.apache.org/r/33867/#comment133632>

    Same as Line:408



common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
<https://reviews.apache.org/r/33867/#comment133636>

    This method can be made more robust and generic by checking for both 
staging and working path so that tests for staging directory alone can also use 
it.


- Ajay Yadava


On May 6, 2015, 10:38 p.m., Balu Vellanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33867/
> -----------------------------------------------------------
> 
> (Updated May 6, 2015, 10:38 p.m.)
> 
> 
> Review request for Falcon, Sowmya Ramesh and Venkat Ranganathan.
> 
> 
> Bugs: Falcon-1195
>     https://issues.apache.org/jira/browse/Falcon-1195
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> testClusterWithOnlyStaging fails intermittently due to race condition in 
> creating working dir for cluster. This can be fixed by creating a different 
> working/staging dir for each test case.
> 
> 
> Diffs
> -----
> 
>   
> common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 
> 4555cb0 
>   
> common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java
>  4920d03 
> 
> Diff: https://reviews.apache.org/r/33867/diff/
> 
> 
> Testing
> -------
> 
> Ran unit tests multiple times to verify the changes.
> 
> 
> Thanks,
> 
> Balu Vellanki
> 
>

Reply via email to