On Jan 31, 2013, at 6:07 AM, Angelo Dell'Aera <angelo.dellaera@gmail.com> wrote:
On Wed, 30 Jan 2013 23:32:34 +0100 Angelo Dell'Aera <angelo.dellaera@gmail.com> wrote:
On Wed, 30 Jan 2013 11:04:36 -0800 Glyph <glyph@twistedmatrix.com> wrote:
Any volunteers for parts of this process?
I'm not familiar with Twisted patching process and for this reason I'm just attaching a small patch here for #6245 because I'd like to discuss about the approach. If correct I will move on in the process (hopefully in the right way)
The patch simply tries to encode the name argument properly if unicode. This is the same approach used by ralphm but applied to Name class initialization so it should be really generic.
Just about a doubt about how to handle an exception potentially raised during the name encoding. Any idea?
Ciao.
PS Attached a simple test code which forces the name to resolve to be unicode. It fails against 12.3.0 while it is correclty executed after patching.
I read documentation about Twisted testing and tested if the suggested patch introduces some regressions in the existing code
buffer@saiph ~/Twisted-12.3.0/twisted $ trial twisted.names [..] Ran 271 tests in 0.425s
PASSED (successes=271)
which seems like it's not happening.
Obviously this is not exhaustive because seems like there are no specific tests for that code path (name is always passed as byte) but I can try writing some additional ones if needed.
Hi Angelo, Thanks for your contribution. It looks like the ticket in question already has a branch in review though, so we won't need your patch this time. In the future, the right way to submit patches is to attach them to the appropriate ticket; in this case, <http://twistedmatrix.com/trac/ticket/6245>. They tend to get lost or ignored on the mailing list (as happened here). To answer your question about additional tests, yes, whenever we fix a bug in Twisted, the fix has to come along with a new test to ensure that it stays fixed in subsequent releases. Thanks, -glyph