pvary commented on code in PR #16611:
URL: https://github.com/apache/iceberg/pull/16611#discussion_r3466082014


##########
core/src/test/java/org/apache/iceberg/formats/TestFormatModelRegistry.java:
##########
@@ -86,6 +88,77 @@ void testFailingReRegistrations() {
         .hasMessageContaining("Cannot register class");
   }
 
+  @Test
+  void registerDoesNotFailWhenClassThrowsNoClassDefFoundOnInvoke() {

Review Comment:
   This test essentially **copies** the relevant logic from the production code 
and verifies the outcome of those calls. The problem is that if we change 
anything in `FormatModelRegistry` or `InternalData`, the test will silently 
become stale because it **copies** the implementation rather than validating 
its behavior independently.
   
   While modifying production code purely for testability is not always the 
most elegant approach, I think extracting testable methods is a reasonable and 
widely accepted trade-off. It gives us a more robust test that remains aligned 
with the actual implementation instead of relying on copied logic that will 
drift over time.



-- 
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]

Reply via email to