----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41568/#review112548 -----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 818) <https://reviews.apache.org/r/41568/#comment173038> wrong exception documentation falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 307) <https://reviews.apache.org/r/41568/#comment173040> It is usually a best practice to name the test in a manner that it makes the purpose of the test clear. You may want to rename it to something like "testTooFrequentRetentionLifecycleStage" falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 369) <https://reviews.apache.org/r/41568/#comment173045> I think instead of dynamically configuring them with boolean, they should be part of different tests. This is preparation of test data and this shouldn't be configured dynamically based on the flags from data provider. I believe the motivation is maximum reuse of data and you should be able to do that with 2 functions - setGlobalRetention, setLocalRetention and calling the various combinations of these in your test scenarios. this will make the code easier to read and maintain. falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 397) <https://reviews.apache.org/r/41568/#comment173048> These if else cases in tests make the test code difficult to read and maintain. This will also go away if you try to refactor it into different tests leveraging common methods for test data preparation. falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 416) <https://reviews.apache.org/r/41568/#comment173047> This is not data, these are flags to prepare the data, so this is not exactly the best way to use data provider. - Ajay Yadava On Dec. 18, 2015, 9:44 p.m., PRAGYA MITTAL wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41568/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2015, 9:44 p.m.) > > > Review request for Falcon and Ajay Yadava. > > > Bugs: FALCON-1567 > https://issues.apache.org/jira/browse/FALCON-1567 > > > Repository: falcon-git > > > Description > ------- > > Add test cases for https://issues.apache.org/jira/browse/FALCON-965 > > > Diffs > ----- > > > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java > ae96044 > > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java > 8f45d1c > > Diff: https://reviews.apache.org/r/41568/diff/ > > > Testing > ------- > > Tested. > > > Thanks, > > PRAGYA MITTAL > >
