[issue20544] Use specific asserts in operator tests
Roundup Robot added the comment: New changeset 7bf3d91f8d6c by Antoine Pitrou in branch 'default': Closes #20544: use specific asserts in operator tests. https://hg.python.org/cpython/rev/7bf3d91f8d6c -- nosy: +python-dev resolution: - fixed stage: commit review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Serhiy Storchaka added the comment: Backporting this to older releases is needed to help backporting future tests. We should keep tests consistent if possible. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Raymond Hettinger added the comment: I don't think kind of shallow changes to the test suite should be backported. They probably shouldn't have been done at all. When you change code, the tests are a safety net. When you change tests, you have almost no safety net at all. If the original test was developed using test-driven-development or for a bug-fix, then it was demonstrated to have failed at one time. But when pointlessly refactored, the revised tests have no longer been demonstrated to actually detect to original underlying bug. If the whole point of specific asserts is to have better error messages on a failure, then we should wait until there is an actual failure to make the change. Otherwise, this reduced the reliability of the test suite with zero benefit. Most of these tests will never become broken (I can't recall the last time any of them failed) and if they did, their existing message would suffice. Another reason this should be done is Guido's notion of holistic refactoring. The idea is that someone shouldn't travel through modules making shallow changes. That work should be deferred to the module maintainer to be done in the course of normal deeply thought out work on the module. The last reason these kind of changes shouldn't be made is that undoes work by the person who originally wrote it, making the code less familiar to them when they come back. For example, most of the tests for sets are written in a way that reflects how I was thinking about the problem when I wrote it. If someone comes through and renames the variables, rewraps the lines, switches the choice of assertions, etc. Then they make it more difficult to recover the original line of thinking by the designer. Put another way, no one likes to have code they're written needlessly scrambled around. If there is a real bug fix, then yes. Otherwise, shallow wholesale search and replace missions should be approached reluctantly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Antoine Pitrou added the comment: Everyone except Raymond seems to agree the patch is a good think, so it should probably be applied. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Robert Collins added the comment: Looks sane to me. Should go in 3.6 if we're going to do this or get closed to remove cognitive overhead in the issue tracker. No point backporting this to older releases. -- nosy: +rbcollins versions: +Python 3.6 -Python 2.7, Python 3.4, Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Robert Collins added the comment: So, is this specific patch ok to apply, or are we going to reject it? I don't particularly care either way, but having this issue open and stalled just adds cognitive load to working with the bug tracker. FWIW I agree that it should not be backported. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Raymond Hettinger added the comment: Antoine, I'm surprised, usually you're the first to oppose backporting anything other than bug-fixes and were generally supportive of Guido's admonitions about holistic refactoring. It seems you feel strongly enough about the patch to warrant telling all the other participants to simply dismiss the concerns of another senior developer, so go ahead and apply it. But, you should have strong reservations about backporting -- that seems completely unjustified. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Changes by Antoine Pitrou pit...@free.fr: -- assignee: - serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Vincent Davis added the comment: Looks like this is ready to be applied and closed or just closed. -- nosy: +Vincentdavis ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Mark Lawrence added the comment: There are 28 dependencies listed on #16510. 18 have already been closed, presumably with patches committed but I haven't checked them all. 10 are still open including this one. Can we have a decision now as to whether we move forward with committing all of these changes, subject of course to formal review, or simply close the outstanding ones as won't fix. -- nosy: +BreamoreBoy versions: +Python 3.5 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Serhiy Storchaka added the comment: I had stopped committing patches for similar issues because there is an opposition against this. See discussion at http://comments.gmane.org/gmane.comp.python.devel/145535. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Antoine Pitrou added the comment: I think these transformations are useful, because on failure they will give you what is the actual value of the compared operands (something which assertTrue is enable to extract). Also they are clearer to read IMO. -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
STINNER Victor added the comment: I agree with Antoine. -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Andrew Svetlov added the comment: Ok. I agree with Teddy but Raymond's arguments make a value also. Thus we need to make any decision and close the issue (and bunch of similar issues). On Sat, May 3, 2014 at 8:38 AM, Terry J. Reedy rep...@bugs.python.org wrote: Terry J. Reedy added the comment: The generic error message for the first is 'False is not true'. Clear but useless. The error message for the second is the more specific 'representation of object) is not None'. Since the expression was supposed to evaluate to None, but did not, it would be helpful to me, at least, to know what it did evaluate to. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Andrew Svetlov added the comment: LGTM. Ping? -- nosy: +asvetlov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Raymond Hettinger added the comment: While this looks harmless, I seriously question whether it is an improvement. For example, how is this any better? -self.assertTrue(operator.setitem(a, 0, 2) is None) +self.assertIsNone(operator.setitem(a, 0, 2)) This error message for the first is already perfectly clear. I don't see anything that warrants the code churn. Also remember that changing tests is hazardous. We don't have tests for the tests. So, if a test gets damaged, we won't know about it. The particular patch seems fine, but the whole exercise of going through the test suite and altering the tests is a dubious. The odds of us getting ANY value out of this is vanishingly small. -- nosy: +rhettinger ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Terry J. Reedy added the comment: The generic error message for the first is 'False is not true'. Clear but useless. The error message for the second is the more specific 'representation of object) is not None'. Since the expression was supposed to evaluate to None, but did not, it would be helpful to me, at least, to know what it did evaluate to. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Ezio Melotti added the comment: LGTM. -- nosy: +ezio.melotti stage: patch review - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
Terry J. Reedy added the comment: Rietveld's within-line diff highlighting really helps reviewing this sort of thing. LGTM. -- nosy: +terry.reedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue20544] Use specific asserts in operator tests
New submission from Serhiy Storchaka: The proposed patch makes the operator module tests use more specific asserts. This will provide more useful failure report. -- components: Tests files: test_operator_asserts.patch keywords: easy, patch messages: 210540 nosy: serhiy.storchaka priority: normal severity: normal stage: patch review status: open title: Use specific asserts in operator tests type: enhancement versions: Python 2.7, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file33969/test_operator_asserts.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20544 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
