You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Ajay Yadava <aj...@gmail.com> on 2016/01/04 12:51:58 UTC
Re: 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/#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 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
>
>