kbendick commented on code in PR #4734:
URL: https://github.com/apache/iceberg/pull/4734#discussion_r868338155


##########
data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilter.java:
##########
@@ -297,7 +297,9 @@ public void testNoNulls() {
   }
 
   @Test
-  public void testIsNaN() {
+  public void testIsNaNForParquet() {

Review Comment:
   EDIT - I see now why the name was changed.
   
   My suggestion, to avoid modifying the name / making two tests that are named 
specifically for their file format, that you consider making two separate 
functions and  calling out to each situationally.
   
   Like
   ```java
   public void testIsNaN() {
     Assume.assumeTrue("Avro files do not have row group statistics", format != 
FileFormat.AVRO);
     
     if (format == FileFormat.ORC) {
       testIsNanForOrc();
     } else {
       testIsNanForParquet();
     }
   }
   
   private void testIsNanForOrc() { .... }
   
   private void testIsNanForParquet() { ... }
   ```
   
   This way, the same test (by name) will be run for each, but the correct 
assertions will be handled.



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