Message257576
The attached patch fixes the one wording change from the patch2 review, and makes all the error messages consistent as suggested by Serhiy, and also adds a lot more tests: every way that a special method can fall back should now be tested. (Of course it would be good if someone else went through and made sure I didn't miss anything.) However:
> Without tests you can't be sure that there is no special case in some abstract classes or operators, or that it can't be introduced in future.
Yes, but there's clearly a cost to maintaining and running 13 copies of each __ispam__ test, and 20 copies of each __rspam__ test. And the benefit is minuscule--if all 13 methods share the same code; it's far more likely that someone will break the __isub__ test than that they'll break the __isub__ code.
> But I don't know what is the best place for tests. Tests for special methods are scattered though different files: test_binop, test_class, test_compare, test_descr, test_richcmp, test_augassign, test_contains, test_iter, etc.
Yeah, they pretty much have to be scattered around the code. As they are in the attached diff. But I don't think that's a huge problem, except for the poor sap who has to review the patch. :) |
|
Date |
User |
Action |
Args |
2016-01-06 02:03:51 | abarnert | set | recipients:
+ abarnert, gvanrossum, rhettinger, ncoghlan, r.david.murray, martin.panter, serhiy.storchaka |
2016-01-06 02:03:48 | abarnert | set | messageid: <1452045828.9.0.43353216351.issue25958@psf.upfronthosting.co.za> |
2016-01-06 02:03:48 | abarnert | link | issue25958 messages |
2016-01-06 02:03:48 | abarnert | create | |
|