adutra commented on code in PR #13223:
URL: https://github.com/apache/iceberg/pull/13223#discussion_r2130005166


##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -949,14 +951,37 @@ public void testLoadMetadataTable() {
     assertThat(table.name()).isEqualTo(catalog.name() + "." + metaIdent);
   }
 
-  @Test
-  public void testLoadMissingTable() {
+  @ParameterizedTest
+  @MethodSource("nonExistentTableIdentifiers")
+  public void testLoadMissingTable(TableIdentifier identifier) {
     C catalog = catalog();
 
-    assertThat(catalog.tableExists(TBL)).as("Table should not 
exist").isFalse();
-    assertThatThrownBy(() -> catalog.loadTable(TBL))
+    assumeThat(supportsEmptyNamespace() || !identifier.namespace().isEmpty())

Review Comment:
   I see what you mean, but I think it would introduce too many special cases: 
when confronted with invalid identifiers, some catalogs throw IAE in 
`tableExists` (BigQuery), some others throw IAE at `buildTable` (Hive), some 
others throw NSTE at `buildTable` (REST). It will be hard to come up with a 
single test for all catalogs.
   
   And OTOH, if the catalog clearly doesn't support some specific 
configuration, its behavior under such configuration shouldn't be required to 
be normative, imo.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to