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