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

Reply via email to