R. David Murray added the comment: I'm afraid I don't remember the details of our conversations at PyCon. Specifically, I don't remember what problem(s) it is you are trying to solve with this patch.
What is the reason for not just adding your new tests to the ThreadedNetworkTests test case? That way they would be run both in regular mode and SSL mode. You seem to have copied bits of the code from that test into yours, but it mostly moves the code away from our usual conventions. That is, we generally put the support classes at the top level and then subclass them in test methods that need variations, rather than defining them in setup methods (where they would inaccessible unless assigned to self). Also, the existing reap_server method is to be preferred over a chain of addCleanups. Finally, we are moving all tests to using unittest.main() so that they can be called using unittest only instead of requiring regrtest, so the change in the __main__ block is incorrect. Now, all that said, I can see several things I'd do to refactor the existing tests to make them clearer and easier to modify. For example, there are better ways to get reap_threads called on all test methods than wrapping each one individually (ex: tearDownClass). And I prefer setup methods that do addCleanup (in this case, addCleanup(self.reap_server), rather than the existing context manager approach, though I don't know how widespread my preferred pattern is in our test suite yet. All of which has little to do with the title of this issue :). So, perhaps we should open a new issue for refactoring the tests, and then come back to this one once we are both satisfied with the refactoring? We can cite this issue as the motivation for the refactoring. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue22137> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
