pvary commented on code in PR #9212: URL: https://github.com/apache/iceberg/pull/9212#discussion_r1420131007
########## flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/shuffle/TestDataStatisticsOperator.java: ########## @@ -119,9 +121,9 @@ public void testProcessElement() throws Exception { testHarness = createHarness(this.operator)) { StateInitializationContext stateContext = getStateContext(); operator.initializeState(stateContext); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("a")))); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("a")))); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("b")))); + operator.processElement(new StreamRecord<>(genericRowDataA)); + operator.processElement(new StreamRecord<>(genericRowDataA)); Review Comment: Sorry - my midnight review was not clear enough. 😢 I was trying to suggest to process a different `RowData` object, with the same key, like: ``` private final GenericRowData genericRowDataA_1 = GenericRowData.of(StringData.fromString("a"), 1); private final GenericRowData genericRowDataA_2 = GenericRowData.of(StringData.fromString("a"), 2); private final GenericRowData genericRowDataB = GenericRowData.of(StringData.fromString("b"), 3); [..] operator.processElement(new StreamRecord<>(genericRowDataA_1)); operator.processElement(new StreamRecord<>(genericRowDataA_2)); operator.processElement(new StreamRecord<>(genericRowDataB)); ``` I know that we have individual test for the correct grouping, but I consider this as an e2e tests, and it would be nice to test this out as well. ########## flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/shuffle/TestDataStatisticsOperator.java: ########## @@ -119,9 +121,9 @@ public void testProcessElement() throws Exception { testHarness = createHarness(this.operator)) { StateInitializationContext stateContext = getStateContext(); operator.initializeState(stateContext); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("a")))); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("a")))); - operator.processElement(new StreamRecord<>(GenericRowData.of(StringData.fromString("b")))); + operator.processElement(new StreamRecord<>(genericRowDataA)); + operator.processElement(new StreamRecord<>(genericRowDataA)); Review Comment: Sorry - my midnight review was not clear enough. 😢 I was trying to suggest to process a different `RowData` object, with the same key, like: ``` private final GenericRowData genericRowDataA_1 = GenericRowData.of(StringData.fromString("a"), 1); private final GenericRowData genericRowDataA_2 = GenericRowData.of(StringData.fromString("a"), 2); private final GenericRowData genericRowDataB = GenericRowData.of(StringData.fromString("b"), 3); [..] operator.processElement(new StreamRecord<>(genericRowDataA_1)); operator.processElement(new StreamRecord<>(genericRowDataA_2)); operator.processElement(new StreamRecord<>(genericRowDataB)); ``` I know that we have individual test for the correct grouping, but I consider this as an e2e tests, and it would be nice to test this out as well. -- 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