lucasbru commented on code in PR #21722:
URL: https://github.com/apache/kafka/pull/21722#discussion_r2940833348


##########
tests/kafkatest/tests/streams/streams_standby_replica_test.py:
##########
@@ -48,8 +48,12 @@ def __init__(self, test_context):
 
     @cluster(num_nodes=10)
     @matrix(metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_standby_tasks_rebalance(self, metadata_quorum, group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   I would keep this parametrization.



##########
tests/kafkatest/tests/streams/streams_relational_smoke_test.py:
##########
@@ -94,8 +94,13 @@ def __init__(self, test_context):
     @cluster(num_nodes=8)
     @matrix(crash=[False, True],
             metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_streams(self, crash, metadata_quorum, group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(crash=[False, True],
+            metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   Remove this parametrization.



##########
tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py:
##########
@@ -46,8 +46,12 @@ def setUp(self):
 
     @cluster(num_nodes=7)
     @matrix(metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_streams_resilient_to_broker_down(self, metadata_quorum, 
group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   remove parametrization



##########
tests/kafkatest/tests/streams/streams_smoke_test.py:
##########
@@ -50,8 +50,14 @@ def __init__(self, test_context):
     @matrix(processing_guarantee=['exactly_once_v2', 'at_least_once'],
             crash=[True, False],
             metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_streams(self, processing_guarantee, crash, metadata_quorum, 
group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(processing_guarantee=['exactly_once_v2', 'at_least_once'],
+            crash=[True, False],
+            metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   Yes, the important thing is that most tests are run in the default 
configuration (batching enabled). 
   
   For system tests, we could have with batching disabled:
   
    - streams_smoke_test
    - streams_broker_bounce_test (but reduce the number of parameters, don't 
run all of them)
   
   All other with batching enabled. For integration tests, right now batching 
is disabled by default in this PR. so I would keep all integration tests. If we 
enable batching by default in integration tests, I think it would be okay to 
just keep:
   
    - RebalanceProtocolMigrationTest
    - RebalanceIntegrationTest
    - SmokeTestDriverIntegrationTest



##########
tests/kafkatest/tests/streams/streams_static_membership_test.py:
##########
@@ -49,8 +49,9 @@ def __init__(self, test_context):
                                            acks=1)
 
     @cluster(num_nodes=8)
-    @matrix(metadata_quorum=[quorum.isolated_kraft])
-    def 
test_rolling_bounces_will_not_trigger_rebalance_under_static_membership(self, 
metadata_quorum):
+    @matrix(metadata_quorum=[quorum.isolated_kraft],
+            enable_assignment_batching=[False, True])

Review Comment:
   this one doesn't use streams protocol, so I would not parametrize it.



##########
tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py:
##########
@@ -156,8 +164,12 @@ def test_streams_runs_with_broker_down_initially(self, 
metadata_quorum, group_pr
 
     @cluster(num_nodes=9)
     @matrix(metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_streams_should_scale_in_while_brokers_down(self, metadata_quorum, 
group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   remove parametrization



##########
tests/kafkatest/tests/streams/streams_broker_down_resilience_test.py:
##########
@@ -85,8 +89,12 @@ def test_streams_resilient_to_broker_down(self, 
metadata_quorum, group_protocol)
 
     @cluster(num_nodes=7)
     @matrix(metadata_quorum=[quorum.combined_kraft],
-            group_protocol=["classic", "streams"])
-    def test_streams_runs_with_broker_down_initially(self, metadata_quorum, 
group_protocol):
+            group_protocol=["classic"],
+            enable_assignment_batching=[False])
+    @matrix(metadata_quorum=[quorum.combined_kraft],
+            group_protocol=["streams"],
+            enable_assignment_batching=[False, True])

Review Comment:
   remove parametrization



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to