Copilot commented on code in PR #20004:
URL: https://github.com/apache/kafka/pull/20004#discussion_r2158445191
##########
tests/kafkatest/tests/streams/streams_application_upgrade_test.py:
##########
@@ -252,7 +253,8 @@ def do_stop_start_bounce(self, processor, upgrade_from,
new_version, counter):
second_other_monitor.wait_until(self.processed_msg,
timeout_sec=60,
err_msg="Never saw output '%s'
on " % self.processed_msg + str(second_other_node.account))
- node.account.ssh_capture("grep SMOKE-TEST-CLIENT-CLOSED %s" %
processor.STDOUT_FILE, allow_fail=False)
+ # some older versions crash on shutdown, so we allow crashes here.
+ node.account.ssh_capture("grep -E
'SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)' %s" % processor.STDOUT_FILE,
allow_fail=False)
Review Comment:
[nitpick] Consider centralizing the regex pattern
'SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)' into a constant to maintain consistency
and ease potential future modifications.
```suggestion
node.account.ssh_capture("grep -E %s %s" % (SMOKE_TEST_CLIENT_REGEX,
processor.STDOUT_FILE), allow_fail=False)
```
##########
tests/kafkatest/tests/streams/streams_application_upgrade_test.py:
##########
@@ -163,9 +163,10 @@ def restart_all_nodes_with(self, version):
self.processor3.stop_node(self.processor3.node)
# make sure the members have stopped
- self.wait_for_verification(self.processor1,
"SMOKE-TEST-CLIENT-CLOSED", self.processor1.STDOUT_FILE)
- self.wait_for_verification(self.processor2,
"SMOKE-TEST-CLIENT-CLOSED", self.processor2.STDOUT_FILE)
- self.wait_for_verification(self.processor3,
"SMOKE-TEST-CLIENT-CLOSED", self.processor3.STDOUT_FILE)
+ # some older versions crash on shutdown, so we allow crashes here.
+ self.wait_for_verification(self.processor1,
"SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)", self.processor1.STDOUT_FILE)
+ self.wait_for_verification(self.processor2,
"SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)", self.processor2.STDOUT_FILE)
+ self.wait_for_verification(self.processor3,
"SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)", self.processor3.STDOUT_FILE)
Review Comment:
[nitpick] Consider extracting the regex pattern
'SMOKE-TEST-CLIENT-(EXCEPTION|CLOSED)' into a constant variable to reduce
duplication and simplify future updates in this test.
```suggestion
self.wait_for_verification(self.processor1,
self.SMOKE_TEST_CLIENT_EXCEPTION_OR_CLOSED, self.processor1.STDOUT_FILE)
self.wait_for_verification(self.processor2,
self.SMOKE_TEST_CLIENT_EXCEPTION_OR_CLOSED, self.processor2.STDOUT_FILE)
self.wait_for_verification(self.processor3,
self.SMOKE_TEST_CLIENT_EXCEPTION_OR_CLOSED, self.processor3.STDOUT_FILE)
```
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
