This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2014年02月07日 20:25 by serhiy.storchaka, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| test_bigmem_asserts.patch | serhiy.storchaka, 2014年02月07日 20:25 | review | ||
| test_bigmem_asserts_2.patch | serhiy.storchaka, 2014年02月14日 12:01 | review | ||
| test_bigmem_asserts_3.patch | serhiy.storchaka, 2017年04月05日 05:42 | review | ||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 791 | closed | serhiy.storchaka, 2017年03月23日 17:21 | |
| Messages (6) | |||
|---|---|---|---|
| msg210544 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2014年02月07日 20:25 | |
The proposed patch makes bigmem tests use more specific asserts. This will provide more useful failure report. |
|||
| msg211197 - (view) | Author: Raymond Hettinger (rhettinger) * (Python committer) | Date: 2014年02月14日 02:15 | |
These should be backported. And it probably shouldn't be done at all unless there is an actual failure with an uninformative error message. Otherwise, you're just destabilizing the test suite and creating unnecessary code churn. In the case of the collections tests, I used test-driven-development for parts of it and am very confident in the test as they stand. If you start switching the test methods, I become less confident in those tests (i.e. I haven't seen the new ones fail in the absence of the code they were meant to test). Additionally, the "more specific tests" introduce some additional opacity that is harmful for knowing what methods and operators are specifically used internally in test method. For end users of Python, they don't have to worry much about this, but we as developers of core types really care whether self.assertLessThan(x, y) really does x < y, or x.__lt__(y), or "not y >= x", etc. IOW, I am of the strong opinion that your patches are not a good idea. The "more specific tests" can be used in new tests or in tests that are failing, but going back and making blanket sweeps of the test suite isn't a good practice. Please lookup Guido's comments on "holistic refactoring" being preferred to these kind of "sweeps". |
|||
| msg211216 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2014年02月14日 12:01 | |
> Additionally, the "more specific tests" introduce some additional opacity > that is harmful for knowing what methods and operators are specifically > used internally in test method. For end users of Python, they don't have > to worry much about this, but we as developers of core types really care > whether self.assertLessThan(x, y) really does x < y, or x.__lt__(y), or > "not y >= x", etc. If the test explicitly designed to test relation or boolean operations, I leave it as is. I try to be very careful. After more careful reviewing this patch, I have found than some tests shouldn't be changed, because they produce utterly large error message in case of a failure (even if resulted message is truncated, as in case of assertEqual, large intermediate strings are created). Here is corrected patch. |
|||
| msg211218 - (view) | Author: Raymond Hettinger (rhettinger) * (Python committer) | Date: 2014年02月14日 12:29 | |
> I try to be very careful. It's great to be careful, but it is a smarter move to not change the test suite at all (except in cases where there is a known problem). There is almost zero benefit to the patch (i.e. the tests are currently not failing and have been stable for a long time). You risk making a mistake, leaving an undetected hole in the tests, and increasing chances of future regression during normal maintenance. Further, this patch churns the code away from what the original test case authors were thinking about when they were deeply engaged with the code. That is why Guido wants only "holistic" refactorings. Another issue is that if you make the extensive test suite changes, there is no case for backporting those changes (especially for Python 2.7). But then, if the versions don't line up, it makes cross-version maintenance more difficult if an actual bug arises (i.e. the patches won't apply cleanly). In short, I recommend that you please don't do this. It isn't good for the project. Thank you. |
|||
| msg211257 - (view) | Author: Terry J. Reedy (terry.reedy) * (Python committer) | Date: 2014年02月15日 03:58 | |
Patch2 looks correct, given that the replacement is the right thing to do. The assertTrue error message 'False is not true' is definitely not helpful. But I see Raymond's point. I think these patches should be discussed on pydev. |
|||
| msg211260 - (view) | Author: Ezio Melotti (ezio.melotti) * (Python committer) | Date: 2014年02月15日 06:37 | |
> For end users of Python, they don't have to worry much about this, but > we as developers of core types really care whether self.assertLessThan(x, y) > really does x < y, or x.__lt__(y), or "not y >= x", etc. FWIW the assert methods should guarantee that the corresponding operator is used (e.g. < for assertLess), and I think this is already the case. > After more careful reviewing this patch, I have found than some tests > shouldn't be changed, because they produce utterly large error > message in case of a failure (even if resulted message is truncated, > as in case of assertEqual, large intermediate strings are created). Some of these cases are already fixed, for others there are still open issues. If you find cases that are not tracked you should report them. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:58 | admin | set | github: 64746 |
| 2017年04月05日 05:43:00 | serhiy.storchaka | set | status: open -> closed files: + test_bigmem_asserts_3.patch resolution: rejected stage: patch review -> resolved |
| 2017年03月23日 17:21:30 | serhiy.storchaka | set | pull_requests: + pull_request696 |
| 2014年02月15日 06:37:38 | ezio.melotti | set | nosy:
+ ezio.melotti messages: + msg211260 |
| 2014年02月15日 03:58:20 | terry.reedy | set | nosy:
+ terry.reedy messages: + msg211257 |
| 2014年02月14日 12:29:17 | rhettinger | set | messages: + msg211218 |
| 2014年02月14日 12:01:11 | serhiy.storchaka | set | files:
+ test_bigmem_asserts_2.patch messages: + msg211216 |
| 2014年02月14日 02:16:01 | rhettinger | set | nosy:
+ rhettinger messages: + msg211197 |
| 2014年02月07日 20:29:32 | serhiy.storchaka | link | issue16510 dependencies |
| 2014年02月07日 20:25:56 | serhiy.storchaka | create | |