kgiusti commented on a change in pull request #1441:
URL: https://github.com/apache/qpid-dispatch/pull/1441#discussion_r751355815
##########
File path: tests/system_tests_distribution.py
##########
@@ -3884,7 +3882,7 @@ def __init__(self,
n_links_per_message = 4
self.n_expected_transitions = len(self.senders) *
self.messages_per_sender * n_links_per_message
- self.debug = False
+ self.debug = debug
self.test_name = test_name
Review comment:
Since github won't let me comment on lines that haven't been changed:
this is regards to the debug_print() function: add a flush=True to the print
statement: print(message, flush=True). unittest has a tendency to not flush
stdout if a failure is hit.
##########
File path: tests/system_tests_distribution.py
##########
@@ -3826,6 +3823,7 @@ def __init__(self,
self.error = None
self.messages_per_sender = 100
Review comment:
Non-patch observation: is 100 messages *really* necessary? CI systems
tend to be painfully slow. Can the same degree of confidence that this works
be achieved by say 20 messages?
##########
File path: tests/system_tests_distribution.py
##########
@@ -3910,11 +3908,9 @@ def send_from_client(self, sender, n_messages,
sender_index):
n_sent += 1
self.n_sent += 1
self.n_transitions += 1
- self.debug_print("send_from_client -- sender: %d n_sent: %d" %
(sender_index, n_sent))
+ self.debug_print("send_from_client -- sender: %d n_sent: %d" %
(sender_index, n_sent))
Review comment:
Did you also want to do the same thing (move the debug print out of the
loop) for the send_from_waypoint() method below? Just asking...
##########
File path: tests/system_tests_distribution.py
##########
@@ -3972,19 +3958,25 @@ def on_link_opening(self, event):
if self.n_waypoint_receivers < 2 :
self.waypoints[self.n_waypoint_receivers]['receiver'] =
event.receiver
self.n_waypoint_receivers += 1
+ # Create the senders after the waypoint receiver links have
been opened.
+ if self.n_waypoint_receivers == 2:
+ for i in range(len(self.sender_connections)):
+ cnx = self.sender_connections[i]
+ sender = self.senders[i]
+ sender['sender'] = event.container.create_sender(cnx,
+
self.destination,
+
name=link_name())
+ sender['to_send'] = self.messages_per_sender
+ sender['n_sent'] = 0
def on_sendable(self, event):
- self.debug_print("on_sendable ------------------------------")
for index in range(len(self.senders)) :
sender = self.senders[index]
if event.sender == sender['sender'] :
- self.debug_print(" client sender %d" % index)
if sender['n_sent'] < sender['to_send'] :
self.debug_print(" sending %d" % sender['to_send'])
self.send_from_client(sender['sender'], sender['to_send'],
index)
sender['n_sent'] = sender['to_send'] # n_sent = n_to_send
Review comment:
wait a sec... can't send_from_client exit with < to_send if credit runs
out? This seems wrong. Maybe send_from_client should be incrementing
sender['n_sent'] rather than having this line assume sender['to_send'] was
actually sent.
And don't get me started on why the heck this test is using maps
(sender['n_sent']) rather than a class (sender.n_sent) all over this test.
It's like this test was purposely written to be easy to break and maximize
reader confusion.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
