kbendick commented on code in PR #5037:
URL: https://github.com/apache/iceberg/pull/5037#discussion_r920345031
##########
nessie/src/test/java/org/apache/iceberg/nessie/TestNessieTable.java:
##########
@@ -356,6 +360,81 @@ public void testListTables() {
Assertions.assertThat(catalog.tableExists(TABLE_IDENTIFIER)).isTrue();
}
+ private void testRegister(TableIdentifier identifier, String
metadataVersionFiles) {
Review Comment:
> I do not like the duplicated test code, and I think it would be good to
have a general Catalog API test for checking the functionality of the new
Catalog implementations. Something like TestCatalog but for all/most of the
catalog API methods. We can do it in a different PR, but it would be good to
have them. The test for registerTable() is a good candidate for these common
tests.
We do have `CatalogTests`, which is what `TestRESTCatalog` is built off of.
It’s nice in that it has some methods for subclasses to declare if the catalog
being tested supports certain features, so individual tests can exit early if
they don’t support an optional feature (similar to JUnit `Assume`).
For example if the catalog in question doesn’t support namespace properties.
Might be good to see if that can be made to fit this need or can otherwise
get some ideas from it. I believe Nessie also implements these tests.
--
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]
