nastra commented on code in PR #9793:
URL: https://github.com/apache/iceberg/pull/9793#discussion_r1584312612
##########
data/src/test/java/org/apache/iceberg/data/TestMetricsRowGroupFilterTypes.java:
##########
@@ -282,28 +286,33 @@ public TestMetricsRowGroupFilterTypes(
this.skipValue = skipValue;
}
- @Test
+ @TestTemplate
public void testEq() {
- boolean shouldRead = shouldRead(readValue);
- Assert.assertTrue("Should read: value is in the row group: " + readValue,
shouldRead);
+ boolean shouldRead = shouldRead(readValue, column);
+ Assertions.assertThat(shouldRead).as("Should read: value is in the row
group: " + readValue)
+ .isTrue();
- shouldRead = shouldRead(skipValue);
- Assert.assertFalse("Should skip: value is not in the row group: " +
skipValue, shouldRead);
+ shouldRead = shouldRead(skipValue, column);
+ Assertions.assertThat(shouldRead).as("Should skip: value is not in the row
group: " + skipValue)
+ .isFalse();
}
- private boolean shouldRead(Object value) {
- switch (format) {
- case ORC:
- return shouldReadOrc(value);
- case PARQUET:
- return shouldReadParquet(value);
- default:
- throw new UnsupportedOperationException(
- "Row group filter types tests not supported for " + format);
+ private boolean shouldRead(Object value, String column) {
Review Comment:
I don't see a reason why we'd need to add the `column` as a parameter. I
think this should be reverted (same for the other methods)
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
