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.
* We need to create a CustomSessionCatalog in advance, wrap it with
spy, and return it in `newSessionCatalog`.
3. Test the right thing: The boolean verifies that our custom implementation
runs. `newTableOps()` will be called either way (can't differentiate whether
the default or custom is invoked or not), so `verify()` doesn't actually prove
customization worked.
4. 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]
