singhpk234 commented on code in PR #16310:
URL: https://github.com/apache/iceberg/pull/16310#discussion_r3235748856
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -4011,9 +3376,91 @@ protected RESTSessionCatalog newSessionCatalog(
return catalog;
}
+ static HTTPRequest reqMatcher(HTTPMethod method) {
+ return argThat(req -> req.method() == method);
+ }
+
+ static HTTPRequest reqMatcher(HTTPMethod method, String path) {
+ return argThat(req -> req.method() == method && req.path().equals(path));
+ }
+
+ static HTTPRequest reqMatcher(HTTPMethod method, String path, Map<String,
String> headers) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers)));
+ }
+
+ static HTTPRequest reqMatcher(
+ HTTPMethod method, String path, Map<String, String> headers, Map<String,
String> parameters) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers))
+ && req.queryParameters().equals(parameters));
+ }
+
+ static HTTPRequest reqMatcher(
+ HTTPMethod method,
+ String path,
+ Map<String, String> headers,
+ Map<String, String> parameters,
+ Object body) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers))
+ && req.queryParameters().equals(parameters)
+ && Objects.equals(req.body(), body));
+ }
+
private static List<HTTPRequest> allRequests(RESTCatalogAdapter adapter) {
ArgumentCaptor<HTTPRequest> captor =
ArgumentCaptor.forClass(HTTPRequest.class);
verify(adapter, atLeastOnce()).execute(captor.capture(), any(), any(),
any());
return captor.getAllValues();
}
+
+ @Test
+ public void closeCascadesToCustomMetricsReporter() throws IOException {
+ CloseTrackingMetricsReporter.CLOSE_COUNT.set(0);
+
+ RESTCatalogAdapter adapter = Mockito.spy(new
RESTCatalogAdapter(backendCatalog));
+ RESTCatalog catalog =
+ new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config)
-> adapter);
+ catalog.initialize(
+ "prod",
+ ImmutableMap.of(
+ CatalogProperties.URI,
+ "ignored",
+ CatalogProperties.METRICS_REPORTER_IMPL,
+ CloseTrackingMetricsReporter.class.getName()));
+
+ catalog.close();
+
+ assertThat(CloseTrackingMetricsReporter.CLOSE_COUNT.get())
+ .as("RESTCatalog.close() must propagate to the configured
MetricsReporter")
+ .isEqualTo(1);
+ }
+
+ /**
+ * Test-only MetricsReporter that tracks how many times {@code close()} was
called via a static
+ * counter (the reporter is loaded reflectively, so per-instance state is
not directly reachable
+ * from the test).
+ */
+ public static class CloseTrackingMetricsReporter
+ implements org.apache.iceberg.metrics.MetricsReporter {
Review Comment:
+1 lets avoid inline import !
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -4011,9 +3376,91 @@ protected RESTSessionCatalog newSessionCatalog(
return catalog;
}
+ static HTTPRequest reqMatcher(HTTPMethod method) {
+ return argThat(req -> req.method() == method);
+ }
+
+ static HTTPRequest reqMatcher(HTTPMethod method, String path) {
+ return argThat(req -> req.method() == method && req.path().equals(path));
+ }
+
+ static HTTPRequest reqMatcher(HTTPMethod method, String path, Map<String,
String> headers) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers)));
+ }
+
+ static HTTPRequest reqMatcher(
+ HTTPMethod method, String path, Map<String, String> headers, Map<String,
String> parameters) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers))
+ && req.queryParameters().equals(parameters));
+ }
+
+ static HTTPRequest reqMatcher(
+ HTTPMethod method,
+ String path,
+ Map<String, String> headers,
+ Map<String, String> parameters,
+ Object body) {
+ return argThat(
+ req ->
+ req.method() == method
+ && req.path().equals(path)
+ && req.headers().equals(HTTPHeaders.of(headers))
+ && req.queryParameters().equals(parameters)
+ && Objects.equals(req.body(), body));
+ }
+
private static List<HTTPRequest> allRequests(RESTCatalogAdapter adapter) {
ArgumentCaptor<HTTPRequest> captor =
ArgumentCaptor.forClass(HTTPRequest.class);
verify(adapter, atLeastOnce()).execute(captor.capture(), any(), any(),
any());
return captor.getAllValues();
}
+
+ @Test
+ public void closeCascadesToCustomMetricsReporter() throws IOException {
+ CloseTrackingMetricsReporter.CLOSE_COUNT.set(0);
+
+ RESTCatalogAdapter adapter = Mockito.spy(new
RESTCatalogAdapter(backendCatalog));
+ RESTCatalog catalog =
+ new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config)
-> adapter);
+ catalog.initialize(
+ "prod",
+ ImmutableMap.of(
+ CatalogProperties.URI,
+ "ignored",
+ CatalogProperties.METRICS_REPORTER_IMPL,
+ CloseTrackingMetricsReporter.class.getName()));
+
+ catalog.close();
+
+ assertThat(CloseTrackingMetricsReporter.CLOSE_COUNT.get())
+ .as("RESTCatalog.close() must propagate to the configured
MetricsReporter")
+ .isEqualTo(1);
+ }
+
+ /**
+ * Test-only MetricsReporter that tracks how many times {@code close()} was
called via a static
+ * counter (the reporter is loaded reflectively, so per-instance state is
not directly reachable
+ * from the test).
+ */
+ public static class CloseTrackingMetricsReporter
Review Comment:
can we just enhance the `CatalogTests.CustomMetricsReporter` to record the
close rather than using new one ?
--
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]
