On Mar 25, 2009, at 20:44, James Hawkins truiken@gmail.com wrote:
On Wed, Mar 25, 2009 at 11:50 AM, Paul Vriens paul.vriens.wine@gmail.com wrote:
James Hawkins wrote:
On Wed, Mar 25, 2009 at 8:25 AM, Paul Vriens <paul.vriens.wine@gmail.com
wrote:
Hi,
This fixes bug 17843 but I'm not sure it's a 100% correct. James didn't change this just for the fun of it.
If you're unsure of the correct fix, you should write a test case that fails without your patch and succeeds with your patch.
AJ already put in a fix (similar to mine).
I don't want to sound negative but you added loads of test cases that still didn't prevent this regression.
That is pretty negative. Your comment implies that the failure of the current test suite to be comprehensive enough means we shouldn't add more tests, which is incorrect. No test suite is ever complete. Any change to the code that can be tested, should be tested, and there are very few places that can't be tested with enough ingenuity.
That's probably the downside of todo_wine.
With my last patch in the series (which needs to be reworked and more tests added for), there were only two todo_wine's left, and they are unrelated to this problem.
-- James Hawkins
Sorry if that came on too strong. I'm all for more tests. My point was that todo_wine can be a 'dangerage' thing sometimes (not specifically in this case).
Maybe we should even make it mandatory that fixes for regressions should be accompanied with tests, although I doubt that will be feasible.
Paul