[GitHub] [kafka] AlanConfluent commented on a diff in pull request #12458: MINOR: Adds KRaft versions of most streams system tests

[GitBox <gi...@apache.org>]

AlanConfluent commented on code in PR #12458:
URL: https://github.com/apache/kafka/pull/12458#discussion_r938317255


##########
tests/kafkatest/tests/streams/streams_broker_bounce_test.py:
##########
@@ -205,11 +212,17 @@ def collect_results(self, sleep_time_secs):
         return data
 
     @cluster(num_nodes=7)
+    @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"],
+            broker_type=["leader"],
+            num_threads=[1, 3],
+            sleep_time_secs=[120],
+            metadata_quorum=[quorum.remote_kraft])

Review Comment:
   That's what was suggested for tests which are not testing core Kafka functionality and could be affected, which I don't think this is.  For those, the predefined variable `quorum.all_non_upgrade` exists and is equal to `[quorum.remote_kraft, quorum.zk]`.  Also, given how long these tests run, I didn't want to run more variants than is necessary.



##########
tests/kafkatest/tests/streams/streams_broker_bounce_test.py:
##########
@@ -205,11 +211,17 @@ def collect_results(self, sleep_time_secs):
         return data
 
     @cluster(num_nodes=7)
+    @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"],
+            broker_type=["leader"],
+            num_threads=[1, 3],
+            sleep_time_secs=[120],
+            metadata_quorum=[quorum.remote_kraft])
     @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"],
             broker_type=["leader", "controller"],
             num_threads=[1, 3],
             sleep_time_secs=[120])

Review Comment:
   I see.  Done!



##########
tests/kafkatest/tests/streams/streams_broker_bounce_test.py:
##########
@@ -251,8 +264,9 @@ def test_broker_type_bounce_at_start(self, failure_mode, broker_type, sleep_time
 
     @cluster(num_nodes=7)
     @matrix(failure_mode=["clean_shutdown", "hard_shutdown", "clean_bounce", "hard_bounce"],
-            num_failures=[2])
-    def test_many_brokers_bounce(self, failure_mode, num_failures):
+            num_failures=[2],
+            metadata_quorum=quorum.all_non_upgrade)
+    def test_many_brokers_bounce(self, failure_mode, num_failures, metadata_quorum=quorum.zk):

Review Comment:
   Yeah, I think if it's not defined, it'll default to using zk, but I just wanted to be a bit more explicit about the default.



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org