zabetak commented on a change in pull request #1186:
URL: https://github.com/apache/hive/pull/1186#discussion_r447999118



##########
File path: 
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
##########
@@ -136,17 +129,19 @@
     MetaStoreTestUtils.setConfForStandloneMode(conf);
     ObjectStore objectStore = new ObjectStore();
     objectStore.setConf(conf);
-    objectStore.dropTable(DEFAULT_CATALOG_NAME, db1Utbl1.getDbName(), 
db1Utbl1.getTableName());
-    Deadline.startTimer("");
-    objectStore.dropPartitions(DEFAULT_CATALOG_NAME, db1Ptbl1.getDbName(), 
db1Ptbl1.getTableName(), db1Ptbl1PtnNames);
-    objectStore.dropTable(DEFAULT_CATALOG_NAME, db1Ptbl1.getDbName(), 
db1Ptbl1.getTableName());
-    objectStore.dropTable(DEFAULT_CATALOG_NAME, db2Utbl1.getDbName(), 
db2Utbl1.getTableName());
-    Deadline.startTimer("");
-    objectStore.dropPartitions(DEFAULT_CATALOG_NAME, db2Ptbl1.getDbName(), 
db2Ptbl1.getTableName(), db2Ptbl1PtnNames);
-    objectStore.dropTable(DEFAULT_CATALOG_NAME, db2Ptbl1.getDbName(), 
db2Ptbl1.getTableName());
-    objectStore.dropDatabase(DEFAULT_CATALOG_NAME, db1.getName());
-    objectStore.dropDatabase(DEFAULT_CATALOG_NAME, db2.getName());
+    for (String clg : objectStore.getCatalogs()) {

Review comment:
       The `setUp` method adds things in the Metastore that are not necessarily 
needed by every test and this was the case even before my changes. In the new 
test, I added some more tables/columns cause I think it made things easier to 
understand at least for me. If I was to follow the current pattern I would have 
to add the tables, cols, partitions, etc., in the `setUp` method and then drop 
them in the `teardown` method but then they would be visible to all other tests 
as well. 
   
   I like the philosophy where unit tests are minimal (least possible state) 
thus I would prefer if every test starts with a clean Metastore and ends with a 
clean Metastore. I didn't change the `setUp` in order not to introduce too many 
unrelated changes but I made one step forward in the `teardown`.  
   
   If every test in this class was using the same state then I would definitely 
agree that the `setUp` and `teardown` methods should be the way they are right 
now. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to