stevenzwu commented on code in PR #6660: URL: https://github.com/apache/iceberg/pull/6660#discussion_r1088414695
########## flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkIcebergSink.java: ########## @@ -85,28 +86,29 @@ public class TestFlinkIcebergSink { private final int parallelism; private final boolean partitioned; - @Parameterized.Parameters(name = "format={0}, parallelism = {1}, partitioned = {2}") + @Parameterized.Parameters(name = "format={0}, parallelism = {1}, partitioned = {2}, branch = {3}") Review Comment: I have a little bit concern of the multiplication factor of Parameterized tests. I feel we don't need to test different file format, writer parallelism, partitioned or not for the branch write. I am wondering if we should have a separate `TestFlinkIcebergSinkBranching`. it can be parameterized to test both v1 and v2 table format. also we can add more accurate assertion that write happened on the `test_branch` and verified that no snapshots/writes added to the `main` branch. -- 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