Vinay Sajip <vinay_sa...@yahoo.co.uk> added the comment: Thanks for doing this, I appreciate the effort you've put into it. My comments:
1. Each test class ought to be independent, but they aren't - for example, if I comment out all of your test classes other than LoggerTest, it fails. 2. I try and avoid modifying the handler list across tests. Rather than running test_logging.py by itself, it should run cleanly when run using pythonX.Y regrtest.py test_logging test_logging in the Lib/test folder. (That's right, I invoke it twice because that sometimes flushes out bugs which don't show up with a single invocation.) 3. I'm not if it's worth it to monkey-patch stuff (such as the _log method in test_log_invalidLevel), or even patching io.StringIO in test_findCaller. In general more coverage is good, but I also favour being pragmatic about tests. IMO, testing at too low a level can incur unnecessary overhead in re-implementing tests if some internals are refactored. Some of your tests do fall into that category, e.g. checking that the "parent" attribute is polled is an implementation detail, though checking hasHandlers() isn't (in test_hasHandlers_breakOnPropagateFalse). Another example: that logging.disable sets logging.root.manager.disable is an implementation detail, and so not worth checking as much as checking the *effect* of the disable call on logging output. ISTM other tests in the stdlib test suite do not go to the low level that yours do - essentially mocking parts of the system. Although undoubtedly I should incorporate some of your tests to fill in gaps in coverage, I am not sure I want to just accept the patch as it is, for the reasons I have given above. (Notwithstanding that, please understand that I still appreciate the effort you've put into this somewhat unglamorous activity.) As a matter of interest, what was the coverage percentage *before* you made your changes? ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue11332> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
