> On Jan. 8, 2018, 3:15 p.m., Marta Kuczora wrote: > > Thanks a lot for creating this testing infra for HMS testing!!! > > Overall it looks great to me, I would have only a few suggestions regarding > > the database tests. Please see my notes in the file.
Thanks for the review Marta! > On Jan. 8, 2018, 3:15 p.m., Marta Kuczora wrote: > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > > Lines 168 (patched) > > <https://reviews.apache.org/r/65018/diff/1/?file=1935455#file1935455line168> > > > > I think it would be good to split this test into separate ones so each > > of them would test only one use-case, like > > - creating db with null name > > - creating db with invalid name > > - creating db with empty name > > This way it would be easier to see what use-case a test is supposed to > > test. > > What do you think? Done > On Jan. 8, 2018, 3:15 p.m., Marta Kuczora wrote: > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > > Lines 282 (patched) > > <https://reviews.apache.org/r/65018/diff/1/?file=1935455#file1935455line282> > > > > I think this test could be also split (like the > > testCreateDatabaseInvalidData) into two separate tests: > > - drop null db > > - drop default db Done > On Jan. 8, 2018, 3:15 p.m., Marta Kuczora wrote: > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > > Lines 340 (patched) > > <https://reviews.apache.org/r/65018/diff/1/?file=1935455#file1935455line340> > > > > This could also be separated into two tests. > > - drop with ignore unknown parameter true > > - drop with ignore unknown parameter false Done > On Jan. 8, 2018, 3:15 p.m., Marta Kuczora wrote: > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > > Lines 353 (patched) > > <https://reviews.apache.org/r/65018/diff/1/?file=1935455#file1935455line353> > > > > This could also be separated into multiple test cases. Done - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65018/#review194948 ----------------------------------------------------------- On Jan. 9, 2018, 2:50 p.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65018/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2018, 2:50 p.m.) > > > Review request for hive, Alan Gates, Marta Kuczora, Adam Szita, and Vihang > Karajgaonkar. > > > Bugs: HIVE-18372 > https://issues.apache.org/jira/browse/HIVE-18372 > > > Repository: hive-git > > > Description > ------- > > Created: > - AbstractMetastore class - to privide an interface for different metastore > implementation (start/stop/warehouse path methods) > -- Implementation for Embedded/Remote/Cluster metastores > - MiniHMS with builder - to create hms instances for test > - MetaStoreFactory - to create the parameter list for parametrized test > - TestDatabases - test for database related metastore functions to showcase > the infrastructure > > > Diffs > ----- > > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/MetaStoreFactory.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/ClusterMetaStore.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/EmbeddedMetaStore.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/MiniHMS.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/RemoteMetaStore.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65018/diff/2/ > > > Testing > ------- > > Run the new tests > > > Thanks, > > Peter Vary > >
