[Twisted-Python] Ticket #1330 - Socks V5 functionality

[David Stainton]

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,  <exarkun at twistedmatrix.com> wrote:
> On 06:14 pm, dstainton415 at 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 at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python