nastra commented on code in PR #8918:
URL: https://github.com/apache/iceberg/pull/8918#discussion_r1425445565


##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java:
##########
@@ -1211,33 +1183,48 @@ public void 
testDatabaseLocationWithSlashInWarehouseDir() {
   }
 
   @Test
-  public void testRegisterTable() {
-    TableIdentifier identifier = TableIdentifier.of(DB_NAME, "t1");
-    catalog.createTable(identifier, getTestSchema());
-    Table registeringTable = catalog.loadTable(identifier);
-    catalog.dropTable(identifier, false);
-    TableOperations ops = ((HasTableOperations) registeringTable).operations();
-    String metadataLocation = ((HiveTableOperations) 
ops).currentMetadataLocation();
-    Table registeredTable = catalog.registerTable(identifier, 
metadataLocation);
-    assertThat(registeredTable).isNotNull();
-    TestHelpers.assertSerializedAndLoadedMetadata(registeringTable, 
registeredTable);
-    String expectedMetadataLocation =
-        ((HasTableOperations) 
registeredTable).operations().current().metadataFileLocation();
-    assertThat(metadataLocation).isEqualTo(expectedMetadataLocation);
-    assertThat(catalog.loadTable(identifier)).isNotNull();
-    assertThat(catalog.dropTable(identifier)).isTrue();
-  }
+  @Override
+  public void testRenameTableDestinationTableAlreadyExists() {
+    Namespace ns = Namespace.of("newdb");
+    TableIdentifier renamedTable = TableIdentifier.of(ns, "table_renamed");
 
-  @Test
-  public void testRegisterExistingTable() {
-    TableIdentifier identifier = TableIdentifier.of(DB_NAME, "t1");
-    catalog.createTable(identifier, getTestSchema());
-    Table registeringTable = catalog.loadTable(identifier);
-    TableOperations ops = ((HasTableOperations) registeringTable).operations();
-    String metadataLocation = ((HiveTableOperations) 
ops).currentMetadataLocation();
-    assertThatThrownBy(() -> catalog.registerTable(identifier, 
metadataLocation))
-        .isInstanceOf(AlreadyExistsException.class)
-        .hasMessage("Table already exists: hivedb.t1");
-    assertThat(catalog.dropTable(identifier, true)).isTrue();
+    if (requiresNamespaceCreate()) {
+      catalog.createNamespace(ns);
+    }
+
+    Assertions.assertThat(catalog.tableExists(TABLE))

Review Comment:
   please add a TODO and mention the issue that addresses this. Also it's not 
clear what exactly is different from the test in `CatalogTests`, so please add 
a comment



-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

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


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

Reply via email to