wypoon commented on a change in pull request #1508:
URL: https://github.com/apache/iceberg/pull/1508#discussion_r733916531
##########
File path:
spark/v2.4/spark2/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceHiveTables.java
##########
@@ -44,11 +44,14 @@ public static void start() {
@After
public void dropTable() throws IOException {
- Table table = catalog.loadTable(currentIdentifier);
- Path tablePath = new Path(table.location());
- FileSystem fs =
tablePath.getFileSystem(spark.sessionState().newHadoopConf());
- fs.delete(tablePath, true);
- catalog.dropTable(currentIdentifier, false);
+ if (currentIdentifier != null) {
Review comment:
This code used to be in the spark common module before Anton removed it.
On rebase, it was moved to v2.4, and I did not scrutinize what happened as
there were no conflicts to resolve and tests passed.
This is actually a fix for a latent bug that was exposed when I skipped
tests. Every test in `TestIcebergSourceTablesBase` calls `createTable`, which
in this subclass sets the static `currentIdentifier`. At the end of each test,
this `@After` method drops the table identified by `currentIdentifier`. If the
test is skipped, this `@After` method is still run, and will fail because the
table was already dropped after the previous test.
My impetus was to skip the tests I added when testing Spark 3. Now that the
code is duplicated and the v3.0 version of `TestIcebergSourceTablesBase` does
not have the added tests, test skipping using `Assume` is not necessary.
Neverthelss, I'd argue that this is still a bug and I propose to keep the fix
here and add it to v.3.0 as well.
Your thoughts?
--
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
