Hi Jean-Paul, Thanks for all the info, observations about the existing code and for the coding advice!
I think I should only implement the socks 5 server side since txsocksx seems to have the client implementation covered. Some of the Tor developers use it... I'm not used to test driven development. I'll give it a try and implement the SOCKSv5 server functionality... Cheers! David On Sat, Nov 16, 2013 at 11:44 AM, <exar...@twistedmatrix.com> wrote: > On 06:14 pm, dstainton...@gmail.com wrote: >> >> Hi, I'd like to help out and write unit tests for the Socks v5 code in >> this ticket: >> https://twistedmatrix.com/trac/ticket/1330 >> >> Should I write something very similar to this?? :: >> http://twistedmatrix.com/trac/browser/trunk/twisted/test/test_socks.py >> >> My goal is getting socksv5 client and server code merged to mainline >> Twisted with unit tests. > > > > twisted/test/test_socks.py is a bad example of a test suite. Here are the > things about it you should not emulate: > > * It has documentation that is far from complete. Documentation is just as > important in unit tests as elsewhere. In particular, documenting the intent > of every test method is critical otherwise the test suite is very difficult > to maintain. > > * It exercises too much code in each test method. Well written test > methods do a single very simple thing. A good rule of thumb is that there > should only be one `TestCase.assert...` method call in each test method. > > * It uses some `TestCase.assert...` methods which are deprecated or soon to > be deprecated. `assert_` is the main offender here. > > * It doesn't completely cover the implementation (probably because the > implementation wasn't developed test-driven). You can achieve full test > coverage without doing test-driven development but it takes more discipline. > I suggest doing a test-driven implementation of the SOCKSv5 functionality > you want (the easy approach to this is to start writing tests, then copy the > *smallest* possible piece of the existing, untested implementation into your > new implementation to make those tests pass; repeat until you have all of > the desired functionality). > > * `StringTCPTransport` seems redundant. `StringTransport` offers all of > this functionality already. > > * Many names used in the module don't follow the Twisted name convention > (most obviously, "under_scores" are used throughout rather than > "camelCase"). > > * Native strings are used to represent byte strings throughout. > > * The protocol interface is uniformly misused (it should call > `makeConnection` not `connectionMade`) > > Hope this helps, > Jean-Paul > > _______________________________________________ > Twisted-Python mailing list > Twisted-Python@twistedmatrix.com > http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python _______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
