gaborkaszab commented on code in PR #14709:
URL: https://github.com/apache/iceberg/pull/14709#discussion_r2712025933
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -4326,6 +4326,23 @@ public void
testTableCacheAgeDoesNotRefreshesAfterAccess() {
.isEqualTo(HALF_OF_TABLE_EXPIRATION);
}
+ @Test
+ public void testNumLoadTableCallsForMergeAppend() {
+ RESTCatalogAdapter adapter = Mockito.spy(new
RESTCatalogAdapter(backendCatalog));
+
+ RESTCatalog catalog = catalog(adapter);
+
+ catalog.createNamespace(TABLE.namespace());
+
+ BaseTable table = (BaseTable) catalog.createTable(TABLE, SCHEMA);
+
+ table.newAppend().appendFile(FILE_A).commit();
Review Comment:
I added verification of the outgoing CommitReport on the metrics endpoint
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -3271,6 +3271,11 @@ public void testCatalogWithCustomMetricsReporter()
throws IOException {
assertThat(CustomMetricsReporter.SCAN_COUNTER.get()).isEqualTo(1);
// reset counter in case subclasses run this test multiple times
CustomMetricsReporter.SCAN_COUNTER.set(0);
+
+ // append file through MergeAppend and check and reset counter
+ table.newAppend().appendFile(FILE_A).commit();
+ assertThat(CustomMetricsReporter.COMMIT_COUNTER.get()).isEqualTo(1);
+ CustomMetricsReporter.COMMIT_COUNTER.set(0);
Review Comment:
I'm not sure either :) Probably I found some test gap here, but I agree this
addition is unrelated to the intent of the PR. Removing this.
--
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]
