> On Jan. 10, 2018, 6:35 p.m., Vihang Karajgaonkar wrote: > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDatabases.java > > Lines 350 (patched) > > <https://reviews.apache.org/r/65018/diff/3/?file=1938268#file1938268line350> > > > > I think we should catch the exception and assert the exception type is > > InvalidOperationException so that we catch errors like if someone changes > > the thrown exception in the future. > > Peter Vary wrote: > Shall I separate the test case into two? With/Without cascade? There is a > little extra stuff there creating the table/function/index in two tests which > I wanted to avoid with one test case. That is why I decied to keep the > cascade tests in one test case and used try-catch here to check the exception > without the cascade option, and then proceed with the test and drop the > database with the cascade option. > > What do you think?
I'd vote for separation. One test case for cascade and one for without cascade. The latter should have an @Test(expected = ..) annotation IMHO - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65018/#review195158 ----------------------------------------------------------- On Jan. 11, 2018, 12:54 p.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65018/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2018, 12:54 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/MetaStoreFactoryForTests.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/ClusterMetaStoreForTests.java > PRE-CREATION > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/EmbeddedMetaStoreForTests.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/RemoteMetaStoreForTests.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/65018/diff/4/ > > > Testing > ------- > > Run the new tests > > > Thanks, > > Peter Vary > >
