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 2016/01/04 15:46:19 UTC
Re: Review Request 41748: [FALCON-1699] Test fixes for RetentionTest,
LineageApiTest, TouchAPIPrismAndServerTest,
FeedReplicationTest and few fortifications
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41748/#review112579
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 530)
<https://reviews.apache.org/r/41748/#comment173074>
Java doc missing.
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java (line 531)
<https://reviews.apache.org/r/41748/#comment173075>
numberOfRetries can be a parameter of the function itself. User may want to change it according to his/her test case. You can also set 5 as default value and let user override it if he wants.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedReplicationTest.java (line 252)
<https://reviews.apache.org/r/41748/#comment173076>
assertTrue(boolean condition, String message) can be used.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/TouchAPIPrismAndServerTest.java (line 108)
<https://reviews.apache.org/r/41748/#comment173077>
Why is this required?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java (line 190)
<https://reviews.apache.org/r/41748/#comment173078>
TimeUtil.addMinsToTime() can be used instead of this.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java (line 83)
<https://reviews.apache.org/r/41748/#comment173079>
Consider moving this test case out of the code instead of disabling it.
- PRAGYA MITTAL
On Dec. 28, 2015, 7 p.m., Paul Isaychuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41748/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2015, 7 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1699
> https://issues.apache.org/jira/browse/FALCON-1699
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Test fixes for RetentionTest, LineageApiTest, TouchAPIPrismAndServerTest, FeedReplicationTest and few fortifications
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/lineage/Edge.java c1a7eb8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/KerberosHelper.java c9f540f
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java ae96044
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/request/BaseRequest.java e5430eb
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/FeedReplicationTest.java 6728edf
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/TouchAPIPrismAndServerTest.java 1bffe9a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/hcat/HCatRetentionTest.java d639c21
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java 8f45d1c
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java 73273f9
>
> Diff: https://reviews.apache.org/r/41748/diff/
>
>
> Testing
> -------
>
> tested
>
>
> Thanks,
>
> Paul Isaychuk
>
>
Re: Review Request 41748: [FALCON-1699] Test fixes for RetentionTest,
LineageApiTest, TouchAPIPrismAndServerTest,
FeedReplicationTest and few fortifications
Posted by Paul Isaychuk <pi...@hortonworks.com>.
> On Jan. 4, 2016, 2:46 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/TouchAPIPrismAndServerTest.java, line 108
> > <https://reviews.apache.org/r/41748/diff/1/?file=1176943#file1176943line108>
> >
> > Why is this required?
We got a failure because of unstable touch response message. It was either "Old bundle...Old coordinator...New bundle id: 00012..B" or "Old bundle...Old coordinator id...New coordinator id: 00012...C" (unstable part of message was the last sentence: New bundle id.../New coordinator id...). As figured out it depends on ether bundle had coordinators or not. So we added wait to assure that bundle has coordinators.
> On Jan. 4, 2016, 2:46 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java, line 191
> > <https://reviews.apache.org/r/41748/diff/1/?file=1176945#file1176945line191>
> >
> > TimeUtil.addMinsToTime() can be used instead of this.
We need to just subtract 10 seconds not minute (note that integer points to milliseconds). Even if we could use TimeUtil instead, we were forced to create date as string first:
String date = TimeUtil.dateToOozieDate(new DateTime(DateTimeZone.UTC).toDate());
And then subtract amount of minutes and cast it back to DateTime, like:
TimeUtil.oozieDateToDate(TimeUtil.addMinsToTime(date, 1));
Which seems not much readable.
> On Jan. 4, 2016, 2:46 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java, line 84
> > <https://reviews.apache.org/r/41748/diff/1/?file=1176946#file1176946line84>
> >
> > Consider moving this test case out of the code instead of disabling it.
We disabled it not because test is not required, but because we had a lot of pain with it, and even last fix with kerberos switch didn't work as we expected affecting other tests. So to make this test not breaking other tests we disabled it for now. So I Suppose we might return to fix it later if this test case is needed.
> On Jan. 4, 2016, 2:46 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java, line 531
> > <https://reviews.apache.org/r/41748/diff/1/?file=1176940#file1176940line531>
> >
> > numberOfRetries can be a parameter of the function itself. User may want to change it according to his/her test case. You can also set 5 as default value and let user override it if he wants.
changed method as requested, tested it with: FeedReplicationTest, FeedDelayTest, FeedLateRerunTest.
- Paul
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41748/#review112579
-----------------------------------------------------------
On Jan. 5, 2016, 7:15 p.m., Paul Isaychuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41748/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2016, 7:15 p.m.)
>
>
> Review request for Falcon.
>
>
> Bugs: FALCON-1699
> https://issues.apache.org/jira/browse/FALCON-1699
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Test fixes for RetentionTest, LineageApiTest, TouchAPIPrismAndServerTest, FeedReplicationTest and few fortifications
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/response/lineage/Edge.java c1a7eb8
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/KerberosHelper.java c9f540f
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/OozieUtil.java ae96044
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/request/BaseRequest.java e5430eb
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/TouchAPIPrismAndServerTest.java 1bffe9a
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/hcat/HCatRetentionTest.java d639c21
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/lineage/ListFeedInstancesTest.java 17725ae
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/prism/RetentionTest.java 8f45d1c
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/security/FalconClientTest.java 73273f9
>
> Diff: https://reviews.apache.org/r/41748/diff/
>
>
> Testing
> -------
>
> tested
>
>
> Thanks,
>
> Paul Isaychuk
>
>