You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by PRAGYA MITTAL <mi...@gmail.com> on 2015/12/18 22:44:28 UTC

Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

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

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


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

> On Jan. 4, 2016, 11:51 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 818
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line818>
> >
> >     wrong exception documentation

Done.


> On Jan. 4, 2016, 11:51 a.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 307
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line307>
> >
> >     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"

Done.


- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

> On Dec. 21, 2015, 6:57 a.m., pavan kumar kolamuri wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 834
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line834>
> >
> >     Just adding along with sandeep comments make sure always atleast pass empty configuration object, since NP might occur when this method is called

Done.


> On Dec. 21, 2015, 6:57 a.m., pavan kumar kolamuri wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 304
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line304>
> >
> >     Make it submit a feed having retention frequency less than 1 hour it will fail. Since in minutes also some one can give minutes(120)

I am submitting feed with frequency 15 mins. I think that will cover the scenario.


- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

> On Dec. 21, 2015, 6:57 a.m., pavan kumar kolamuri wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 361
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line361>
> >
> >     Move this to method since all methods need this in case of lifecycle

This test case covers all the cases. Also that way I have to move every single statement to a method.


- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41568/#review111443
-----------------------------------------------------------



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 834)
<https://reviews.apache.org/r/41568/#comment171643>

    Just adding along with sandeep comments make sure always atleast pass empty configuration object, since NP might occur when this method is called



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 304)
<https://reviews.apache.org/r/41568/#comment171639>

    Make it submit a feed having retention frequency less than 1 hour it will fail. Since in minutes also some one can give minutes(120)



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 361)
<https://reviews.apache.org/r/41568/#comment171640>

    Move this to method since all methods need this in case of lifecycle



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 391)
<https://reviews.apache.org/r/41568/#comment171642>

    If result is passed with Data provider then we can simply have one if and else . If for success and else for failure



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 415)
<https://reviews.apache.org/r/41568/#comment171641>

    Pass result along with data then in test method we can check against result.


- pavan kumar kolamuri


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

> On Dec. 28, 2015, 9:41 a.m., Karishma Gulati wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 24
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line24>
> >
> >     Avoid using '*' import.

Done.


- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41568/#review111964
-----------------------------------------------------------



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 24)
<https://reviews.apache.org/r/41568/#comment172261>

    Avoid using '*' import.


- Karishma Gulati


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.

> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 826
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line826>
> >
> >     coordinatorJob.getAppName().startsWith("FALCON_FEED_RETENTION")

Will do it.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 828
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line828>
> >
> >     return null upon if its not retention

Handled null at else.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 829
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line829>
> >
> >     why return null? don't consider for else at all.
> >     
> >     Return null at the end if retention coordinator not found

If coord is not there then null is returned and it comes out od the function. If retention coord is found durther processing is done.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 304
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line304>
> >
> >     comment is unclear.
> >     Do you mean 
> >     having minutely lifecycle retention frequency.
> >     It would fail since lifecycle retention frequency has to be >= 1 hour.

That is what I meant :) Will make it more clear.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 311
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line311>
> >
> >     latearrival and cutoff not required for this test

lateArrival of feed should be less than or equal to feed's retention limit. By defualt it is set to hours(6) in our template. Since its a minutely retention job so iI configured the late arrival for the same.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 361
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line361>
> >
> >     same here

lateArrival of feed should be less than or equal to feed's retention limit. By defualt it is set to hours(6) in our template. Since its a hourly retention job so iI configured the late arrival for the same.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 373
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line373>
> >
> >     } else if (!globalWithStage) { 
> >     same as else ?

Will implement else.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 382
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171409#file1171409line382>
> >
> >     } else if (!clusterWithStage) { 
> >     same as else ?

Will implement else.


> On Dec. 18, 2015, 10:32 p.m., sandeep samudrala wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 827
> > <https://reviews.apache.org/r/41568/diff/1/?file=1171408#file1171408line827>
> >
> >     why create coord again?

Will fix.


- PRAGYA


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


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41568/#review111281
-----------------------------------------------------------



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 826)
<https://reviews.apache.org/r/41568/#comment171375>

    coordinatorJob.getAppName().startsWith("FALCON_FEED_RETENTION")



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 827)
<https://reviews.apache.org/r/41568/#comment171377>

    why create coord again?



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 828)
<https://reviews.apache.org/r/41568/#comment171378>

    return null upon if its not retention



falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 829)
<https://reviews.apache.org/r/41568/#comment171379>

    why return null? don't consider for else at all.
    
    Return null at the end if retention coordinator not found



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 304)
<https://reviews.apache.org/r/41568/#comment171381>

    comment is unclear.
    Do you mean 
    having minutely lifecycle retention frequency.
    It would fail since lifecycle retention frequency has to be >= 1 hour.



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 311)
<https://reviews.apache.org/r/41568/#comment171392>

    latearrival and cutoff not required for this test



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 361)
<https://reviews.apache.org/r/41568/#comment171393>

    same here



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 373)
<https://reviews.apache.org/r/41568/#comment171395>

    } else if (!globalWithStage) { 
    same as else ?



falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 382)
<https://reviews.apache.org/r/41568/#comment171396>

    } else if (!clusterWithStage) { 
    same as else ?


- sandeep samudrala


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
> 
>


Re: Review Request 41568: [FALCON-1567] Test case for Lifecycle feature .

Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41568/#review118640
-----------------------------------------------------------




falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 361)
<https://reviews.apache.org/r/41568/#comment179934>

    This test case covers all the cases. Also that way I have to move every single statement to a method.


- PRAGYA MITTAL


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
> 
>