jiridanek commented on a change in pull request #1512:
URL: https://github.com/apache/qpid-dispatch/pull/1512#discussion_r803827248



##########
File path: tests/system_tests_multi_tenancy.py
##########
@@ -229,6 +231,17 @@ def 
test_16_two_router_anonymous_sender_tenant_on_both(self):
         test.run()
         self.assertIsNone(test.error)
 
+    def _cleanup_link_routes(func):

Review comment:
       The 'traditional' way of using `unittest` would be to do your `setUp()` 
and `tearDown()` in these two methods. (With classes being understood as a way 
to group together tests that share setup and teardown.)
   
   I'd probably try to put the teardown into `LinkRouteTest` itself... but that 
does not have access to `self.routers`, so it is not such a good idea. Maybe 
add method `def tearDown(routers)` to it and call it in every test...
   
   Decorators are too much magic for my simple tastes, I guess.
   
   It's a matter of preference. The decorator is not too hard to understand... 
And if I had unlimited time, enthusiasm, and money to redesign this test "the 
best way", I would not actually know what to do with it right away... So, lgtm, 
just go ahead, I guess?




-- 
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: dev-unsubscr...@qpid.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

Reply via email to