The networktransaction.py log messages need to be unit-tested. The purpose of this report is to be a mini-tutorial on how to unit-test log messages -- now that Python logging has been enabled for webkit-patch.
Created attachment 52293 [details] Proposed patch Hi guys, this is all there is to it! Note that the tearDown() method also asserts that the array of remaining log messages is empty. This checks that there are no unanticipated log messages you *didn't* want to be present. We get this extra check for free since it's in the tearDown(). (Shinichiro, you've already seen this pattern a number of times, so let's let Adam or Eric review it so we know at least one of them gets a chance to see it. Thanks!)
Comment on attachment 52293 [details] Proposed patch Nice, but can we put the setUp and tearDown code into a base class so we don't have to copy/paste it all the time?
I think it looks nice. :)
(In reply to comment #2) > (From update of attachment 52293 [details]) > Nice, but can we put the setUp and tearDown code into a base class so we don't > have to copy/paste it all the time? Yeah, certainly. It crossed my mind, too. It might be a bit heavy for just a couple lines of code, but since Python supports multiple inheritance, I guess we don't risk it presenting a problem down the road should we ever need another base class. (I've never actually needed to use multiple inheritance in Python before.)
Created attachment 52397 [details] Proposed patch 2 Changed the patch to use a base class. It seems possible that we could run into multiple inheritance issues down the road with this change after all. This is because it seems that the base class I created needs to itself inherit from unittest.TestCase (as opposed to the calling class inheriting from both this base class and unittest.TestCase). Otherwise, the setUp() and tearDown() methods of the base class don't seem to get fired. If we ever needed to inherit from unittest.TestCase twice in this way, I'm not sure whether unittest will know to call the setUp() and tearDown() methods of both of the unittest.TestCase super classes.
Comment on attachment 52397 [details] Proposed patch 2 Yeah, we can deal with the inheritance issues if they come up. We've used this pattern before and been happy with it. Thanks for the patch.
Comment on attachment 52397 [details] Proposed patch 2 Clearing flags on attachment: 52397 Committed r56988: <http://trac.webkit.org/changeset/56988>
All reviewed patches have been landed. Closing bug.