In article <1392492250.26338.83831085.39A5ED08@webmail.messagingengine.com>, Benjamin Peterson wrote:

...

Although Raymond approved a patch for test_bigmem [2], his expressed the insistent recommendation not to do this. So I stop committing new reviewed patches. Terry recommended to discuss this in Python-Dev. What are your thoughts? I tend to agree with Raymond. I think such changes are very welcome when

On Sat, Feb 15, 2014, at 10:12 AM, Serhiy Storchaka wrote: the module or tests are otherwise being changed, but on their on constitute unnecessary churn.

+1 Integrity of the test suite and minimizing code churn top any benefits of more specific messages on failures. The expectation is that most tests will never fail so their changed messages will never be seen. For the rare cases when a test does fail, quite often the test was written in a way that will require examination of the code to understand exactly what the test case was intending to test and why it failed. Having a more specific exception message wouldn't help for many tests without further modifications; the key point is to know that the test failed. -- Ned Deily, nad@acm.org

On 16/02/2014 00:22, Nick Coghlan wrote:

...

In article <1392492250.26338.83831085.39A5ED08@webmail.messagingengine.com>, Benjamin Peterson wrote:

...

...

Although Raymond approved a patch for test_bigmem [2], his expressed the insistent recommendation not to do this. So I stop committing new reviewed patches. Terry recommended to discuss this in Python-Dev. What are your thoughts? I tend to agree with Raymond. I think such changes are very welcome when

On Sat, Feb 15, 2014, at 10:12 AM, Serhiy Storchaka wrote: the module or tests are otherwise being changed, but on their on constitute unnecessary churn.

Right, there are a few key problems with large scale style changes to

On 16 February 2014 09:20, Ned Deily wrote: the test suite:

1. The worst case scenario is where we subtly change a test so that it is no longer testing what it is supposed to be testing, allowing the future introduction of an undetected regression. This isn't particularly *likely*, but a serious problem if it happens.

2. If there are pending patches for that module that include new tests, then the style change may cause the patches to no longer apply cleanly, require rework of bug fix and new feature patches to accommodate the style change.

3. Merges between branches may become more complicated (for reasons similar to 2), unless the style change is also applied to the maintenance branches (which is problematic due to 1). I spend a *lot* of time working with the Python test suite on behalf of Jython, so I appreciate the care CPython puts into its testing. To a large extent, tests written for CPython drive Jython development: I suspect I work with a lot more failing tests than anyone here. Where we have a custom test, I often update them from in the latest CPython tests.

Often a test failure is not caused by code I just wrote, but by adding a CPython test or removing a "skip if Jython", and not having written anything yet. While the irrefutable "False is not true" always raises a smile, I'd welcome something more informative. It's a more than a "style" issue. What Nick says above is also not false, as general guidance, but taken as an iron rule seem to argue against concurrent development at all. Don't we manage this change pretty well already? I see little risk of problems 1-3 in the actual proposal, as the changes themselves are 99% of the "drop-in replacement" type: - self.assertTrue(isinstance(x, int)) + self.assertIsInstance(x, int) I found few places, on a quick scan, that risked changing the meaning: they introduce an if-statement, or refactor the expression -- I don't mean they're actually wrong. The point about breaking Serhiy's patch into independent parts will help manage with merging and this risk. The tests are not library code, but their other use is as an example of good practice in unit testing. I pretty much get my idea of Python's test facilities from this work. It was a while before I realised more expressive methods were available. Jeff Jeff Allen

On Sun, Feb 16, 2014 at 12:22 AM, Nick Coghlan wrote:

The practical benefits of this kind of change in the test suite are also highly dubious, because they *only help if the test fails at some point in the future*. At that point, whoever caused the test to fail will switch into debugging mode, and a couple of relevant points apply:

One place where those points don't apply so cleanly is when the test failure is coming from continuous integration and can't easily be reproduced locally (e.g., because there's a problem on a platform you don't have access to, or because it's some kind of threading-related intermittent failure that's exacerbated by the timing conditions on a particular machine). In those situations, an informative error message can easily save significant debugging time. Count me as +1 on the test updates, provided they're done carefully. (And all those I've looked at from Serhiy do indeed look careful.) -- Mark