You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Karishma Gulati <gu...@yahoo.com> on 2015/11/27 19:26:04 UTC
Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/
-----------------------------------------------------------
Review request for Falcon.
Repository: falcon-git
Description
-------
Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
Diffs
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40779/diff/
Testing
-------
Thanks,
Karishma Gulati
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Karishma Gulati <gu...@yahoo.com>.
> On Nov. 30, 2015, 2:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java, line 51
> > <https://reviews.apache.org/r/40779/diff/1/?file=1148010#file1148010line51>
> >
> > inputDataRoot and outputDataRoot ?
We need to delete all test data (created for input and output feeds) as well, which wasn't getting deleted earlier. All our feed templates have these directories as their default data path.
> On Nov. 30, 2015, 2:40 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 536
> > <https://reviews.apache.org/r/40779/diff/1/?file=1148011#file1148011line536>
> >
> > Add to AssertUtil.
It's a very test logic specific assertion.
- Karishma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review108314
-----------------------------------------------------------
On Dec. 17, 2015, 2:50 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:50 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review108314
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java (line 324)
<https://reviews.apache.org/r/40779/#comment167716>
Is it required ?
falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java (line 51)
<https://reviews.apache.org/r/40779/#comment167732>
inputDataRoot and outputDataRoot ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 93)
<https://reviews.apache.org/r/40779/#comment167751>
Merge the test cases as one and try passing the variables using DataProvider.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 94)
<https://reviews.apache.org/r/40779/#comment167728>
embedded group ?
Will this test not run in distributed mode ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 97)
<https://reviews.apache.org/r/40779/#comment167729>
Change date to some time in 2015. Giving entities of 2010 is not a best practise ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 114)
<https://reviews.apache.org/r/40779/#comment167753>
Can it be global ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 115)
<https://reviews.apache.org/r/40779/#comment167754>
Can it be global ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 139)
<https://reviews.apache.org/r/40779/#comment167733>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 180)
<https://reviews.apache.org/r/40779/#comment167734>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 226)
<https://reviews.apache.org/r/40779/#comment167735>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 272)
<https://reviews.apache.org/r/40779/#comment167736>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 289)
<https://reviews.apache.org/r/40779/#comment167737>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 306)
<https://reviews.apache.org/r/40779/#comment167738>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 323)
<https://reviews.apache.org/r/40779/#comment167739>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 340)
<https://reviews.apache.org/r/40779/#comment167740>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 359)
<https://reviews.apache.org/r/40779/#comment167741>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 382)
<https://reviews.apache.org/r/40779/#comment167742>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 412)
<https://reviews.apache.org/r/40779/#comment167743>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 484)
<https://reviews.apache.org/r/40779/#comment167744>
See the comment above ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 536)
<https://reviews.apache.org/r/40779/#comment167750>
Add to AssertUtil.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 537)
<https://reviews.apache.org/r/40779/#comment167749>
Java doc ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 541)
<https://reviews.apache.org/r/40779/#comment167747>
Since thsi is a generic method add this to InstanceUtil so that other test cases can also use it.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 542)
<https://reviews.apache.org/r/40779/#comment167745>
Add Java doc.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 562)
<https://reviews.apache.org/r/40779/#comment167748>
See the comment above.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 563)
<https://reviews.apache.org/r/40779/#comment167746>
Add Java doc.
- PRAGYA MITTAL
On Nov. 27, 2015, 6:26 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2015, 6:26 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Ajay Yadava <aj...@gmail.com>.
> On Dec. 16, 2015, 12:25 p.m., Ajay Yadava wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 58
> > <https://reviews.apache.org/r/40779/diff/1/?file=1148011#file1148011line58>
> >
> > cluster10C , server0C? Any particular reason for this kind of naming convention?
>
> Karishma Gulati wrote:
> cluster1OC stands for cluster's Oozie Client. Changing it to clusterOC. This naming convention is consistent throughout the suite.
Aah, the font confused me whether it was Zero or O. :)
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review110652
-----------------------------------------------------------
On Dec. 17, 2015, 2:50 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:50 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review110652
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java (line 324)
<https://reviews.apache.org/r/40779/#comment170684>
This function doesn't seem to be used anywhere.
What does this function do? It seems it just changes the first output feed to new feed in datasets.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 58)
<https://reviews.apache.org/r/40779/#comment170685>
cluster10C , server0C? Any particular reason for this kind of naming convention?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (lines 96 - 110)
<https://reviews.apache.org/r/40779/#comment170686>
This code seems to be common across tests. Is it possible to extract it out? I am thinking it will reduce the code to core assertions and improve readability. It's ok if it is not possible to extract it out easily or it becomes too clunky.
- Ajay Yadava
On Nov. 27, 2015, 6:26 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2015, 6:26 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/bundle/Bundle.java 67d9ee2
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Karishma Gulati <gu...@yahoo.com>.
> On Dec. 18, 2015, 1:12 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 150
> > <https://reviews.apache.org/r/40779/diff/2/?file=1168681#file1168681line150>
> >
> > Add prism/server in dataprovider and merge prism/server methods.
The prism/server tests belong to different groups - embedded and distributed. So can't be merged.
> On Dec. 18, 2015, 1:12 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 452
> > <https://reviews.apache.org/r/40779/diff/2/?file=1168681#file1168681line452>
> >
> > Add one more arguement to define whether its server/prism. That way you can merge test cases.
Can't, as explained above.
> On Dec. 18, 2015, 1:12 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 208
> > <https://reviews.apache.org/r/40779/diff/2/?file=1168681#file1168681line208>
> >
> > Add the submitSchedule part to beforeMethod.
Some tests are modifying definitions befoe submitting and scheduling.
> On Dec. 18, 2015, 1:12 p.m., PRAGYA MITTAL wrote:
> > falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java, line 112
> > <https://reviews.apache.org/r/40779/diff/2/?file=1168681#file1168681line112>
> >
> > There are already functions available to get entity name based on its type. Consider using them.
Using your second suggestion instead
- Karishma
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review111173
-----------------------------------------------------------
On Dec. 17, 2015, 2:50 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:50 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review111173
-----------------------------------------------------------
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java (line 222)
<https://reviews.apache.org/r/40779/#comment171208>
<p>Assert.assertFailed(triageResult)</p>
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java (line 222)
<https://reviews.apache.org/r/40779/#comment171209>
Assert.assertFailed(triageResult) ?
falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java (line 51)
<https://reviews.apache.org/r/40779/#comment171218>
Not all test cases have these as default input and output directories. There are setInputFeedPath() and setOutputFeedPath() methods available to overwrite the default paths. Although if we want a common input/output path we should set it after baseHDFSDir instaed of different base path so that deleteDirIfExists handles it at one attempt.
falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java (line 130)
<https://reviews.apache.org/r/40779/#comment171219>
Set input/output paths methods are available or you can delete explicitly in your afterMethod/afterClass function. Since its not generic lets avoid hard coding here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 69)
<https://reviews.apache.org/r/40779/#comment171220>
Isn't afterMethod function supposed to handle the clean up?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 80)
<https://reviews.apache.org/r/40779/#comment171221>
Consider setting input/output feed paths here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 101)
<https://reviews.apache.org/r/40779/#comment171263>
This can be a part of beforeMethod ? Specific reason to create extra method.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 102)
<https://reviews.apache.org/r/40779/#comment171262>
set the validity once in beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 103)
<https://reviews.apache.org/r/40779/#comment171261>
Why 3 ? Any specific requirement.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 112)
<https://reviews.apache.org/r/40779/#comment171222>
There are already functions available to get entity name based on its type. Consider using them.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 124)
<https://reviews.apache.org/r/40779/#comment171223>
Define the arguements in Javadoc.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 150)
<https://reviews.apache.org/r/40779/#comment171279>
Add prism/server in dataprovider and merge prism/server methods.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 152)
<https://reviews.apache.org/r/40779/#comment171224>
we one ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 164)
<https://reviews.apache.org/r/40779/#comment171225>
Since you are already checking entity type here, you can get the name directly. No need to add extra method here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 167)
<https://reviews.apache.org/r/40779/#comment171226>
Else is handling process and hence get the process name here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 170)
<https://reviews.apache.org/r/40779/#comment171227>
Assert on succeessful response ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 177)
<https://reviews.apache.org/r/40779/#comment171228>
we one ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 178)
<https://reviews.apache.org/r/40779/#comment171229>
Same doc. Consider changing doc :)
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 186)
<https://reviews.apache.org/r/40779/#comment171230>
There is no difference in the functionality of two methods (just method name is different).
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 189)
<https://reviews.apache.org/r/40779/#comment171231>
Submission via prism should be through prism and not cluster.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 192)
<https://reviews.apache.org/r/40779/#comment171232>
Submission via prism should be through prism and not cluster.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 195)
<https://reviews.apache.org/r/40779/#comment171233>
Assert on success of response.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 208)
<https://reviews.apache.org/r/40779/#comment171264>
Add the submitSchedule part to beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 209)
<https://reviews.apache.org/r/40779/#comment171259>
Add startTime and endTime variable instead of hardcoding here. Also set the validity once in beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 220)
<https://reviews.apache.org/r/40779/#comment171234>
Assert on response status ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 221)
<https://reviews.apache.org/r/40779/#comment171235>
Consider adding assertFailed method in AssertUtil to handle failed response and add check for message there only.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 234)
<https://reviews.apache.org/r/40779/#comment171265>
Configure all this in baseMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 238)
<https://reviews.apache.org/r/40779/#comment171237>
Method name says you are traiging via prism but you are using cluster here.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 244)
<https://reviews.apache.org/r/40779/#comment171236>
Assert on response status ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 253)
<https://reviews.apache.org/r/40779/#comment171267>
embedded ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 257)
<https://reviews.apache.org/r/40779/#comment171266>
Configure all this in baseMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 261)
<https://reviews.apache.org/r/40779/#comment171239>
Assert on response status?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 270)
<https://reviews.apache.org/r/40779/#comment171242>
embedded ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 273)
<https://reviews.apache.org/r/40779/#comment171243>
set concurrency 1 as you are checking for only 1 instance.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 274)
<https://reviews.apache.org/r/40779/#comment171268>
Configure all this in beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 278)
<https://reviews.apache.org/r/40779/#comment171245>
Why checking for success for 3 instances ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 282)
<https://reviews.apache.org/r/40779/#comment171247>
Response status check missing?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 297)
<https://reviews.apache.org/r/40779/#comment171249>
embedded ? Prism will work on distributed also ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 300)
<https://reviews.apache.org/r/40779/#comment171250>
Set concurrency 1
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 301)
<https://reviews.apache.org/r/40779/#comment171269>
Configure all this in beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 305)
<https://reviews.apache.org/r/40779/#comment171251>
Wait on 1 instance, it will reduce overall process time.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 309)
<https://reviews.apache.org/r/40779/#comment171253>
Response status check missing?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 328)
<https://reviews.apache.org/r/40779/#comment171270>
Configure all this in beforeMethod.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 340)
<https://reviews.apache.org/r/40779/#comment171271>
Assert on response status?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 365)
<https://reviews.apache.org/r/40779/#comment171272>
Response status check ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 394)
<https://reviews.apache.org/r/40779/#comment171273>
embedded?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 398)
<https://reviews.apache.org/r/40779/#comment171274>
Move to beforeMethod. Add startTime and endTime variables.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 429)
<https://reviews.apache.org/r/40779/#comment171275>
Assert on response ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 431)
<https://reviews.apache.org/r/40779/#comment171276>
vertext ?
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 452)
<https://reviews.apache.org/r/40779/#comment171277>
Add one more arguement to define whether its server/prism. That way you can merge test cases.
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 459)
<https://reviews.apache.org/r/40779/#comment171278>
Add this to util. As and when more featres will come for triage,we will need these methods to validate response.
- PRAGYA MITTAL
On Dec. 17, 2015, 2:50 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:50 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review111821
-----------------------------------------------------------
Ship it!
Ship It!
- PRAGYA MITTAL
On Dec. 23, 2015, 6:25 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2015, 6:25 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java d8df0fb
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/EntityLineageUtil.java 2df474d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/
-----------------------------------------------------------
(Updated Dec. 23, 2015, 6:25 p.m.)
Review request for Falcon.
Changes
-------
Addressed all review comments.
Repository: falcon-git
Description
-------
Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
Diffs (updated)
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java d8df0fb
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/EntityLineageUtil.java 2df474d
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40779/diff/
Testing
-------
Thanks,
Karishma Gulati
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by PRAGYA MITTAL <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review111777
-----------------------------------------------------------
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java (line 176)
<https://reviews.apache.org/r/40779/#comment172043>
Consider adding response assert after getting response.
- PRAGYA MITTAL
On Dec. 23, 2015, 10:49 a.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 23, 2015, 10:49 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java d8df0fb
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/EntityLineageUtil.java 2df474d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/
-----------------------------------------------------------
(Updated Dec. 23, 2015, 10:49 a.m.)
Review request for Falcon.
Repository: falcon-git
Description
-------
Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
Diffs (updated)
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/AssertUtil.java d8df0fb
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/EntityLineageUtil.java 2df474d
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40779/diff/
Testing
-------
Thanks,
Karishma Gulati
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/#review111168
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Dec. 17, 2015, 2:50 p.m., Karishma Gulati wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40779/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:50 p.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
> As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
>
>
> Diffs
> -----
>
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
> falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
> falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
> falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40779/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Karishma Gulati
>
>
Re: Review Request 40779: [Falcon-1377] Add tests in falcon for the
Triage API
Posted by Karishma Gulati <gu...@yahoo.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40779/
-----------------------------------------------------------
(Updated Dec. 17, 2015, 2:50 p.m.)
Review request for Falcon.
Repository: falcon-git
Description
-------
Add tests for the feature explained in https://issues.apache.org/jira/browse/FALCON-796 - Enable users to triage data processing issues through falcon.
As part of this patch, will add tests corresponding to single colo processing. Cross-colo instance triage tests wil be added in a separate class as part of a separate patch.
Diffs (updated)
-----
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/enumsAndConstants/ResponseErrors.java 921a303
falcon-regression/merlin-core/src/main/java/org/apache/falcon/regression/core/util/InstanceUtil.java 3d05ae9
falcon-regression/merlin/src/main/java/org/apache/falcon/regression/testHelper/BaseTestClass.java 00ef79d
falcon-regression/merlin/src/test/java/org/apache/falcon/regression/triage/TriageAPISingleColoTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/40779/diff/
Testing
-------
Thanks,
Karishma Gulati