> On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > unit/src/test/java/org/apache/falcon/unit/examples/JavaHelloWorldExample.java, > > line 23 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162130#file1162130line23> > > > > Please add more documentation on how to use it and mention some example > > test cases using it. > > pavan kumar kolamuri wrote: > Sorry, i didn't understand. May be comment at wrong place > > Ajay Yadava wrote: > I was just suggesting that the comment is terse and that combined with > name is confusing on purpose of this class. More details in documentation for > purpose and usage examples of this class. e.g. > > This is a dummy class to be used in Java Action of Falcon Unit tests. To > use this... , you can see an example of this at ... > > pavan kumar kolamuri wrote: > I feel all examples should be at same place, based on the use case in > tests users should pick it . For some cases workflow need to be succedded for > that hello world example is used. For some cases workflow should be in > running then sleep example is required. The same example can be used across > different places i don't think it will be good to mention all use cases here.
I didn't get how adding documentation will affect locality of examples. The behaviour that you have mentioned just now is also a good thing to be documented for this class. I feel that the class documentation is the best place for documenting how to use this class. > On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java, > > line 75 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162134#file1162134line75> > > > > Why not use submitAndSchedule, since these are not the focus of this > > test? > > pavan kumar kolamuri wrote: > We want to control no of instances during scheduling process, thats why > used submit and schedule separateley > > Ajay Yadava wrote: > May be, we should make a submitAndSchedule variant which accepts number > of instances. Thoughts? This will make the code much smaller and the core > logic of test more readable. > > pavan kumar kolamuri wrote: > I still feel controlling no of instances should be at scheduling level, > it is not submit level parameter. I agree code should be smaller and core > logic of test more readable , we can refactor later as part of > https://issues.apache.org/jira/browse/FALCON-1659 submitAndSchedule is not just for submit, it is also for schedule and I am assuming it will be a test helper function so shouldn't affect scheduler's public api. It's a small change and is related to code quality of this patch, so I feel it is best to do it in this JIRA. Moreover, I am not sure who will pick it up and when and it might become a bigger tech debt to clear for someone who picks FALCON-1659 later on. - Ajay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40769/#review110174 ----------------------------------------------------------- On Dec. 14, 2015, 7:26 a.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40769/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2015, 7:26 a.m.) > > > Review request for Falcon. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-1447 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1447 > > > Repository: falcon-git > > > Description > ------- > > For Native Scheduler we have tests where all services are mocked and even > jobs are not scheduled in oozie. Integration tests are needed for Native > Scheduler to run from end to end. > > > Diffs > ----- > > oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java > f6e87c4 > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > 2f97c0d > scheduler/pom.xml 336997d > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > ac7cde8 > > scheduler/src/test/java/org/apache/falcon/state/AbstractSchedulerTestBase.java > 48c1426 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 9eb4277 > > unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java > 0065c71 > unit/src/main/resources/oozie-site.xml 23d41eb > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java 2a73516 > > unit/src/test/java/org/apache/falcon/unit/examples/JavaHelloWorldExample.java > PRE-CREATION > webapp/pom.xml 428f67e > > webapp/src/test/java/org/apache/falcon/resource/AbstractSchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/EntitySchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/ProcessInstanceManagerIT.java > 6458b59 > webapp/src/test/java/org/apache/falcon/resource/UnitTestContext.java > 1d49353 > webapp/src/test/resources/helloworldworkflow.xml PRE-CREATION > webapp/src/test/resources/local-process-noinputs-template.xml PRE-CREATION > webapp/src/test/resources/runtime.properties 1da0ca7 > webapp/src/test/resources/startup.properties 756f315 > > Diff: https://reviews.apache.org/r/40769/diff/ > > > Testing > ------- > > These are test cases itself > > > Thanks, > > pavan kumar kolamuri > >
