[Twisted-Python] On testing things wrapping other things
Hi, Recently I worked on refactoring a project so that it was, in my opinion, more testable. For reference, the diff is available at <https://github.com/Weasyl/txyam/compare/Weasyl:9230b7d09c1413a09d776700f26a0...>, and the important parts in the changed version are at <https://github.com/Weasyl/txyam/blob/cdf7aef9e3425f27016f8e97c59bd2143fbf805...>. My question: is this a good factoring for testing a wrapper? It seems to make sense to split apart crafting and issuing requests to clients, as it lets you test the contents of a request to see what will happen instead of having to issue the request and test that its effects happened. On a previous iteration of this project, I wrote tests that did exactly that: issued requests and checked the bytes that were sent to a StringTransport. Those tests ended up being harder to read and write, and seemed to rely on the exact implementation of the thing I was wrapping. Thanks in advance for any feedback on the matter, ~weykent
On Sep 20, 2014, at 11:31 AM, weykent <weykent@weasyl.com> wrote:
Hi,
Recently I worked on refactoring a project so that it was, in my opinion, more testable. For reference, the diff is available at <https://github.com/Weasyl/txyam/compare/Weasyl:9230b7d09c1413a09d776700f26a0...>, and the important parts in the changed version are at <https://github.com/Weasyl/txyam/blob/cdf7aef9e3425f27016f8e97c59bd2143fbf805...>.
My question: is this a good factoring for testing a wrapper? It seems to make sense to split apart crafting and issuing requests to clients, as it lets you test the contents of a request to see what will happen instead of having to issue the request and test that its effects happened.
If anything I'd say that the problem with this code is that it's not a public, documented interface. The clients of this library might want to test it as well :).
On a previous iteration of this project, I wrote tests that did exactly that: issued requests and checked the bytes that were sent to a StringTransport. Those tests ended up being harder to read and write, and seemed to rely on the exact implementation of the thing I was wrapping.
If you're going to check the bytes in a StringTransport, that means you need an implementation of the other end of the protocol as well. If you're just asserting about the exact bytes, then that's really brittle and will break in response to lots of incidental changes. You can use this as a part of a higher-level testing strategy, like so: <https://github.com/rackerlabs/mimic/blob/master/mimic/test/helpers.py> but in this case the unit test code doesn't see the response to its requests until it's been parsed into a sensible data structure, and assertions are about that. -glyph
On Sep 21, 2014, at 10:36 AM, Glyph <glyph@twistedmatrix.com> wrote:
If anything I'd say that the problem with this code is that it's not a public, documented interface. The clients of this library might want to test it as well :).
Ah! You are referring to both _wrap and _issueRequest here, right? That makes sense, and it’s easy to make those documented public interfaces.
If you're going to check the bytes in a StringTransport, that means you need an implementation of the other end of the protocol as well. If you're just asserting about the exact bytes, then that's really brittle and will break in response to lots of incidental changes.
You can use this as a part of a higher-level testing strategy, like so:
<https://github.com/rackerlabs/mimic/blob/master/mimic/test/helpers.py>
but in this case the unit test code doesn't see the response to its requests until it's been parsed into a sensible data structure, and assertions are about that.
Okay, this makes sense. Of course, Twisted doesn’t come with an implementation of a memcache server, and I’d rather not implement one myself for the purpose of testing. I think I’ll stick with my latest approach, then. Would you employ the same approach for testing non-wrappers as well? That is, if you were testing the client implementation of some wire protocol, would you still write parsers for the server implementation of the protocol to make assertions about the sent bytes? As far as I can tell, this is not what, say, the existing memcache protocol tests do.
On Sep 21, 2014, at 10:52 AM, weykent <weykent@weasyl.com> wrote:
On Sep 21, 2014, at 10:36 AM, Glyph <glyph@twistedmatrix.com> wrote:
If anything I'd say that the problem with this code is that it's not a public, documented interface. The clients of this library might want to test it as well :).
Ah! You are referring to both _wrap and _issueRequest here, right? That makes sense, and it’s easy to make those documented public interfaces.
If you're going to check the bytes in a StringTransport, that means you need an implementation of the other end of the protocol as well. If you're just asserting about the exact bytes, then that's really brittle and will break in response to lots of incidental changes.
You can use this as a part of a higher-level testing strategy, like so:
<https://github.com/rackerlabs/mimic/blob/master/mimic/test/helpers.py>
but in this case the unit test code doesn't see the response to its requests until it's been parsed into a sensible data structure, and assertions are about that.
Okay, this makes sense. Of course, Twisted doesn’t come with an implementation of a memcache server, and I’d rather not implement one myself for the purpose of testing. I think I’ll stick with my latest approach, then.
Would you employ the same approach for testing non-wrappers as well? That is, if you were testing the client implementation of some wire protocol, would you still write parsers for the server implementation of the protocol to make assertions about the sent bytes? As far as I can tell, this is not what, say, the existing memcache protocol tests do.
If I were testing the wire protocol itself I'd probably assert about the exact bytes directly. At that level, changes to the exact bytes being emitted are not an incidental change, that's exactly what the system under test is supposed to be doing; producing bytes. So changing those tests would definitely be worthwhile. -glyph
participants (2)
-
Glyph -
weykent