falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul Isaychuk" <pisayc...@hortonworks.com>
Subject Re: Review Request 41748: [FALCON-1699] Test fixes for RetentionTest, LineageApiTest, TouchAPIPrismAndServerTest, FeedReplicationTest and few fortifications
Date Tue, 05 Jan 2016 19:15:49 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message