[Twisted-web] web2 log patch

[glyph at divmod.com]

On Sat, 21 Jan 2006 01:48:33 +0100, Ralf Schmitt <ralf at brainbot.com> wrote:
>glyph at divmod.com schrieb:
>>>I've added a patch for sending new style classes via pb at the end of 
>>>2003.
>>
>>... with no unit tests
>Come on. I added a simple test program. If someone told me to write unit 
>tests for it, I would have done it. But there has just been
>no reaction for 10 months.

Yes.  This is a serious problem.  I just wrote another post which mentions it.  I hope that it does not happen in the future.

I misunderstood your first post and thought that you were aware of the requirement and chose to disregard it - I'm sorry.

>Do the twisted developers only start fixing bugs if the bug reporter writes 
>a unit test revealing that bug?

No.  I could probably argue for a while over whether the bug you submitted was really high-priority; subtle issues opened by the patch; some still-unresolved problems with PB and new-style classes.  It's all pretty much moot, though, and offtopic for this list.

I remember having several in-person exchanges about that _particular_ patch, none of which were recorded in the tracker.  That, also, is a problem, but we have so few in-person exchanges it's hardly worth worrying about.  The context of the unit-tests discussion is this: if you need a fix, and a Twisted developer *doesn't* think it's important, the inclusion of unit tests will likely see it merged regardless.

> (...) I would have written unit tests if someone told me to do it. (...)

Since you had to repeat yourself, let me repeat the apology.  I was confused by the context of your objection.

>I look at the speed of fixing bugs. And in this particular case it took 18 
>months or so to fix it.
>I have added other bug reports, without getting any reaction.

If you'd like to mention them off-list I will have a look and we can discuss the priority they're at and what other possibly undisclosed requirements or requests for more information that there are.  I think tests are important, that doesn't mean I think people should feel ignored by the Twisted team when they're submitting valuable bug reports.

>IMHO, having working code without tests is better than having buggy code 
>without tests (well, one might argue that it's the same...).

I think that the parenthetical comment there pretty much makes my argument for me :).

>I admit that it's nice for the twisted developers to have bug reports in the 
>form of failing unit tests. But you can't expect everyone
>to write them.

Yes.  Let me reiterate - if you discover a serious bug, report it, and it will be fixed, roughly in order of its importance (as determined by some random person on the Twisted project, whose priorities may not be in line with your own).  You don't need to include any tests, or a patch.

*If you have written a patch*, which is seriously important to you, and you want to do what you can to get it merged faster, copious unit tests, and a distant second, a good explanation of what it's for, will get it merged faster.  If you can explain that it is covered by existing tests, and fixes an intermittent failure, that's almost as good.  (For example, I added process support to the default reactor on Windows recently, and there were existing tests which covered that functionality that were previously disabled.  I just turned them on.)

If a Twisted developer chooses to fix your bug, it is almost certain that they will write their *own* tests when they do it, or get endless shit from the rest of us about it :).

>I'm not whining (actually I write unit tests at work). We use twisted at our 
>company and appreciate the work that has gone into it.
>But one thinks about saving that work of reporting bugs, when they just 
>aren't dealt with.

It sounds like we agree on more of this than we disagree on, so I suspect that as I misread your message, you also slightly misread my reply.  People should report bugs, people should send patches.  They won't be harrassed for either, tests or no tests, nor will the patches and bugs be disregarded.  They just shouldn't expect patches with no tests that are not directly in line with a developer's interest to be considered high-priority.

(Of course, on a project with the developer/work ratio of Twisted, anything that isn't high-priority just isn't going to happen.)