XJDKC commented on code in PR #14465:
URL: https://github.com/apache/iceberg/pull/14465#discussion_r2564059639
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3066,6 +3073,101 @@ public void testCommitStateUnknownNotReconciled() {
.satisfies(ex -> assertThat(((CommitStateUnknownException)
ex).getSuppressed()).isEmpty());
}
+ @Test
+ public void testCustomTableOperationsInjection() throws IOException {
+ AtomicBoolean customTableOpsCalled = new AtomicBoolean();
Review Comment:
I prefer using boolean flags over Mockito.verify for this test because:
1. More realistic: The spy approach requires exposing and wrapping the
session catalog instance in `newSessionCatalog()`, which isn't how users would
actually extend these classes.
2. Test the right thing: The boolean verifies that our custom implementation
runs. `newTableOps()` will be called either way (default or custom), so
`verify()` doesn't actually prove customization worked.
3. Simpler and straight forward: set a flag in the override to prove it
executed. No mocking scaffolding needed.
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3066,6 +3073,101 @@ public void testCommitStateUnknownNotReconciled() {
.satisfies(ex -> assertThat(((CommitStateUnknownException)
ex).getSuppressed()).isEmpty());
}
+ @Test
+ public void testCustomTableOperationsInjection() throws IOException {
+ AtomicBoolean customTableOpsCalled = new AtomicBoolean();
Review Comment:
I prefer using boolean flags over `Mockito.verify` for this test because:
1. More realistic: The spy approach requires exposing and wrapping the
session catalog instance in `newSessionCatalog()`, which isn't how users would
actually extend these classes.
2. Test the right thing: The boolean verifies that our custom implementation
runs. `newTableOps()` will be called either way (default or custom), so
`verify()` doesn't actually prove customization worked.
3. Simpler and straight forward: set a flag in the override to prove it
executed. No mocking scaffolding needed.
--
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]
