[GitHub] [iceberg] kbendick commented on a diff in pull request #4734: ORC: Upgrade orc version to use the built-in estimate memory method

[GitBox <gi...@apache.org>]

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


##########
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:
   Nit: You don't need to change the test name, because the `Assume` will take care of it and signal that in the tests.
   
   
   For people that maintain forks and cherry-pick individual changes, updating the test name will probably cause them more of headache than is worth it.
   
   Also, I would consider using `Assume.assumeTrue("Parquet metrics do not track NaN", format == FileFormat.PARQUET)` unless this should also possibly run someday for Avro.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org