kgiusti commented on a change in pull request #1420:
URL: https://github.com/apache/qpid-dispatch/pull/1420#discussion_r750632336
##########
File path: tests/system_tests_two_routers.py
##########
@@ -470,19 +497,17 @@ def on_message(self, event):
'adminStatus': 'deleted'}
request.reply_to =
self.mgmt_receiver_1.remote_source.address
self.mgmt_sender.send(request)
+ self.n_sent += 1
elif event.receiver == self.mgmt_receiver_1:
+ self.n_received += 1
if event.message.properties['statusDescription'] == 'OK' and
event.message.body['adminStatus'] == 'deleted':
Review comment:
I would argue if this response is not OK and deleted the test should
immediately fail.
##########
File path: tests/system_tests_two_routers.py
##########
@@ -439,16 +447,35 @@ def bail(self, error):
def on_link_opened(self, event):
if event.receiver == self.mgmt_receiver:
+ self.mgmt_receiver_link_opened = True
+ elif event.receiver == self.mgmt_receiver_1:
+ self.mgmt_receiver_1_link_opened = True
+ elif event.receiver == self.mgmt_receiver_2:
+ self.mgmt_receiver_2_link_opened = True
+
Review comment:
shouldn't this check also ensure that "receiver_to_kill" link is also
opened? That ensures the "connection_to_kill" exists before the testing starts.
##########
File path: tests/system_tests_two_routers.py
##########
@@ -470,19 +497,17 @@ def on_message(self, event):
'adminStatus': 'deleted'}
request.reply_to =
self.mgmt_receiver_1.remote_source.address
self.mgmt_sender.send(request)
+ self.n_sent += 1
Review comment:
If you add the additional check to wait for "receiver_to_kill", then the
test should immediately fail if this loop exits without finding the connection
to delete.
##########
File path: tests/system_tests_two_routers.py
##########
@@ -409,6 +409,14 @@ def __init__(self, address):
self.mgmt_sender = None
self.success = False
self.error = None
+ self.receiver_to_kill = None
+ self.timer = None
+ self.n_sent = 0
+ self.n_received = 0
+ self.mgmt_receiver_link_opened = False
+ self.mgmt_receiver_1_link_opened = False
+ self.mgmt_receiver_2_link_opened = False
+ self.query_timer = None
def on_start(self, event):
self.timer = event.reactor.schedule(TIMEOUT, TestTimeout(self))
Review comment:
Sorry but github won't let me comment on line 438:
The timeout(self) method should not simply close self.mgmt_conn - it should
call self.bail() with the given error message
##########
File path: tests/system_tests_two_routers.py
##########
@@ -439,16 +447,35 @@ def bail(self, error):
Review comment:
bail() needs to cancel the query timer as well if it is not None
##########
File path: tests/system_tests_two_routers.py
##########
@@ -470,19 +497,17 @@ def on_message(self, event):
'adminStatus': 'deleted'}
request.reply_to =
self.mgmt_receiver_1.remote_source.address
self.mgmt_sender.send(request)
+ self.n_sent += 1
elif event.receiver == self.mgmt_receiver_1:
+ self.n_received += 1
if event.message.properties['statusDescription'] == 'OK' and
event.message.body['adminStatus'] == 'deleted':
- request = Message()
- request.address = "amqp:/_local/$management"
- request.properties = {'type':
'org.apache.qpid.dispatch.connection',
- 'operation': 'QUERY'}
- request.reply_to = self.mgmt_receiver_2.remote_source.address
- self.mgmt_sender.send(request)
+ # Wait for 3 sends for the connection to be gone completely.
+ self.query_timer = event.reactor.schedule(3.0,
PollTimeout(self))
elif event.receiver == self.mgmt_receiver_2:
+ self.n_received += 1
attribute_names = event.message.body['attributeNames']
property_index = attribute_names .index('properties')
- identity_index = attribute_names .index('identity')
for result in event.message.body['results']:
if result[property_index]:
Review comment:
If you're super paranoid (like me) and assume that CI will be
sadistically slow (it will be), instead of calling bail() on line 516, you'd
re-schedule the query_timer and try again. If the connection never gets
deleted then the test will fail when the TIMEOUT timer expires.
--
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]
