> On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 143 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line143> > > > > what are we really testing here? You have already asserted that row for > > entity id doesn't exist, then why call update or delete or get on it?
Just to make sure it is throwing excpetion oigf entity not exists > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 170 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line170> > > > > If you really want to assert the number of rows in result then may be > > it is a good idea to put an assert of 0 results before, at this place. Sure makes sense will add > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 211 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line211> > > > > same as above. AS i said above we want to make sure it should throw exception if entity not exists > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 219 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line219> > > > > same as above. Explained in above comments > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 226 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line226> > > > > same as above. explained > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 233 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line233> > > > > same as above. Explained > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 243 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line243> > > > > Please break it into several small tests instead of one large test. Will do refactor as part of another jira > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 325 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line325> > > > > please create separate tests for each type of operation like delete, > > update etc. Commented > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 340 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line340> > > > > Use equals instead of writing such methods. Yes Ajay Completley Agreed will fix. > On Nov. 5, 2015, 10:50 a.m., Ajay Yadava wrote: > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java, > > line 176 > > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line176> > > > > You should assert the collection with expected collection i.e. assert > > the names as well. Sure will compare, but it will add overhead of maintaining treeset - pavan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39588/#review105229 ----------------------------------------------------------- On Nov. 3, 2015, 5:45 p.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39588/ > ----------------------------------------------------------- > > (Updated Nov. 3, 2015, 5:45 p.m.) > > > Review request for Falcon. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-1234 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1234 > > > Repository: falcon-git > > > Description > ------- > > Persistent State Store for Falcon Native Scheduler > > > Diffs > ----- > > checkstyle/src/main/resources/falcon/checkstyle.xml 2130e73 > checkstyle/src/main/resources/falcon/findbugs-exclude.xml 0a7580d > common/src/main/resources/startup.properties cc5212a > pom.xml 6f2c480 > scheduler/pom.xml 20a91d2 > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java > 3869ff2 > > scheduler/src/main/java/org/apache/falcon/execution/FalconExecutionService.java > b959320 > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java > 19089c4 > scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java fb4ce82 > scheduler/src/main/java/org/apache/falcon/state/ID.java 420c856 > scheduler/src/main/java/org/apache/falcon/state/StateService.java 81357a4 > scheduler/src/main/java/org/apache/falcon/state/store/EntityStateStore.java > 4aa6fdb > > scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java > 3822860 > > scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java > d6a4b49 > scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java > f595c26 > > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java > PRE-CREATION > scheduler/src/main/resources/META-INF/persistence.xml PRE-CREATION > scheduler/src/main/resources/falcon-buildinfo.properties PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java > b2f9e59 > > scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.java > b4a0f35 > > scheduler/src/test/java/org/apache/falcon/state/AbstractSchedulerTestBase.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java > 2f32b43 > > scheduler/src/test/java/org/apache/falcon/state/InstanceStateServiceTest.java > d27ac7e > > scheduler/src/test/java/org/apache/falcon/state/service/TestFalconJPAService.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/tools/TestFalconStateStoreDBCLI.java > PRE-CREATION > scheduler/src/test/resources/startup.properties PRE-CREATION > src/bin/falcon-db.sh PRE-CREATION > src/conf/startup.properties ce6e91f > src/main/assemblies/distributed-package.xml 794eaef > src/main/assemblies/standalone-package.xml fcff8d7 > unit/src/main/resources/startup.properties fe6f430 > > Diff: https://reviews.apache.org/r/39588/diff/ > > > Testing > ------- > > I have written unit tests. I will also test externally by setting up > everything > > > Thanks, > > pavan kumar kolamuri > >
