lliangyu-lin commented on code in PR #12718:
URL: https://github.com/apache/iceberg/pull/12718#discussion_r2032183554
##########
aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java:
##########
@@ -343,28 +342,32 @@ public void testRenameTableFailsToCreateNewTable() {
public void testRenameTableFailsToDeleteOldTable() {
String namespace = createNamespace();
String tableName = createTable(namespace);
- // delete the old table metadata, so that drop old table will fail
+
+ // Simulate table was dropped during rename, where new table already
created and need to delete
+ // old table
+ GlueCatalog glueCatalogSpy = Mockito.spy(glueCatalog);
Review Comment:
I found a issue in my previous test scenario. [DropTable
implementation](https://github.com/apache/iceberg/blob/4bc8ac75efe0d33cc847991b2d99271235e10350/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L364-L366)
will actually swallow the `EntityNotFoundException`, it simply returns false
instead of throwing an exception.
This means the test wasn't actually going into the [rollback codepath in
rename](https://github.com/apache/iceberg/blob/4bc8ac75efe0d33cc847991b2d99271235e10350/aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java#L427-L442),
which only triggers when an exception is thrown.
I've revised the test to simulate a timeout during the drop table API call
instead, which does throw an exception and properly tests the rollback codepath.
--
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
