Bug 36958 – webkitpy: unit test the networktransaction log messages

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Jerdonek]

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.

[Chris Jerdonek]

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!)

[Adam Barth]

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?

[Eric Seidel (no email)]

I think it looks nice. :)

[Chris Jerdonek]

(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.)

[Chris Jerdonek]

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.

[Adam Barth]

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.

[Chris Jerdonek]

Comment on attachment 52397 [details]
Proposed patch 2

Clearing flags on attachment: 52397

Committed r56988: <http://trac.webkit.org/changeset/56988>

[Chris Jerdonek]

All reviewed patches have been landed.  Closing bug.