RussellSpitzer commented on code in PR #11064: URL: https://github.com/apache/iceberg/pull/11064#discussion_r1840963359
########## core/src/test/java/org/apache/iceberg/view/ViewCatalogTests.java: ########## @@ -107,6 +107,7 @@ public void basicCreateView() { assertThat(view.currentVersion().operation()).isEqualTo("create"); assertThat(view.schemas()).hasSize(1).containsKey(0); assertThat(view.versions()).hasSize(1).containsExactly(view.currentVersion()); + assertThat(view.properties()).containsEntry("key1", "catalog-default-key1"); Review Comment: Minor nit in these tests, I don't think it's clear from looking at this test where the value is coming from. I would at least add a comment `// Validate Catalog View Default` Another thing we could do is just make a full test that just checks that catalog defaults are set It would also be nice if we could set the default in the test class rather than in the parent. Not sure if this is possible but something like ```java Create catalog or Set catalog-default properties for test Create view Check if view has default set ``` -- 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