[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Serhiy Storchaka added the comment: Thank you for your contribution Daniel. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: - fixed stage: commit review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Serhiy Storchaka storch...@gmail.com: -- assignee: michael.foord - serhiy.storchaka versions: +Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Roundup Robot added the comment: New changeset 84d7ec21cc43 by Serhiy Storchaka in branch 'default': Issue #15836: assertRaises(), assertRaisesRegex(), assertWarns() and https://hg.python.org/cpython/rev/84d7ec21cc43 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Serhiy Storchaka added the comment: Applied to 3.5 only, because this issue looks rather as new feature (preventing possible user error) and there is minimal chance to break existing tests (see issue24134). -- type: behavior - enhancement versions: -Python 2.7, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Serhiy Storchaka added the comment:
Core developers left a couple of notes and the patch itself is outdated. Here
is updated patch that addresses all comments. It also extends the checking to
assertRaisesRegex(), assertWarns() and assertWarnsRegex().
There is a risk to break existing incorrect tests if the argument is a tuple.
They can be passed for now because caught exception or warning is found before
incorrect value. For example:
with self.assertRaises((ValueError, None)):
int('a')
--
Added file: http://bugs.python.org/file39426/issue15836_2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15836
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Martin Panter added the comment: I posted a question on Reitveld, but the new patch looks fine in general. I wouldn’t worry too much about the (ValueError, None) case, since such code is probably already broken. If it is a problem though, maybe this could only go into the next feature relase (3.5). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Serhiy Storchaka added the comment: Ping. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Berker Peksag added the comment: Since the patch has been reviewed by several core developers, I think you can go ahead and commit it. I'm +0 on the 2.7 version of the patch (the isinstance(e, types.ClassType) part looks fine, but I haven't tested it). It's probably not worth to change anything on the 2.7 branch now :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Michael Foord added the comment: The change to unittest is fine. I'd prefer the tests tweaking as Ezio suggested. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Michael Foord added the comment: I like the first variant suggested by Ezio as more concise. I'll try and look at the substance of the patch today. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Serhiy Storchaka added the comment: I'm +0.5 for the variant suggested by Berker and Ezio. Do you have time to look at the patch Michael? I could commit modified patch (there is one defect in tests). -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Raymond Hettinger raymond.hettin...@gmail.com: -- versions: -Python 2.7 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Martin Panter vadmium...@gmail.com: -- nosy: +vadmium ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Berker Peksag berker.pek...@gmail.com: -- nosy: +berker.peksag stage: patch review - commit review versions: +Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: Ezio requested I comment on his suggestion: I still prefer the try/except form, but I don't feel strongly about it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Claudiu Popa added the comment: This seems to be a reasonable fix. Michael, could you have a look at this patch, please? -- nosy: +Claudiu.Popa versions: +Python 3.5 -Python 3.2, Python 3.3, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Michael Foord added the comment: The patch is just waiting for me to look over it and commit. I'll get to it ASAP. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Daniel Wagner-Hall added the comment: Is anything blocking this patch's submission? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Ezio Melotti added the comment: Would using assertRaises to test assertRaises in the tests be to meta? -- assignee: - michael.foord ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: Ezio: I don't really care whether or not it would be too meta, if you look at the two versions, it is a *lot* clearer what is being tested in the try/except version than it is in the assertRaises version. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Ezio Melotti added the comment: I missed the initial patch. What I was thinking about was to use simply with self.assertRaises(TypeError): self.assertRaises(1) instead of: +ctx = self.assertRaises(TypeError) +with ctx: +self.assertRaises(1) +self.assertIsInstance(ctx.exception, TypeError) or +try: +self.assertRaises(1) +self.fail(Expected TypeError) +except TypeError: +pass Unless I'm missing something, all these should be equivalent. You could even use assertRaisesRegex to check the error message if you think that's appropriate. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Daniel Wagner-Hall added the comment: Cool, my contributor agreement has been received, please merge if happy! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Michael Foord added the comment: Yep, certainly worth fixing. When 3.3 is out the door I will look at applying this to all branches. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by Ezio Melotti ezio.melo...@gmail.com: -- nosy: +ezio.melotti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: Since it is a bugfix it can be applied at any time now. Checkins to default will end up in 3.3.1 and 3.4. (Only features need to wait until after 3.3 is branched in the main repo.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
New submission from Daniel Wagner-Hall: The following code in a unittest test is a no-op: self.assertRaises(lambda: 1) I would expect this to fail the test, because I naively assumed omitting the exception class would act as: self.assertRaises(BaseException, lambda: 1) verifying that *any* Exception is raised. I believe the correct behaviour is to raise a TypeError if excClass is not a BaseException-derived type, similar to if a non-type is passed as the first arg to issubclass. Attached is a patch to do so. It also removes a no-op self.assertRaises from libimport's tests (because it started failing when I ran the tests with the patch). That assertion is redundant, because the two lines above perform the assertion being attempted. -- components: Tests files: assertRaises.patch keywords: patch messages: 169596 nosy: illicitonion priority: normal severity: normal status: open title: unittest assertRaises should verify excClass is actually a BaseException class type: behavior versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 3.4 Added file: http://bugs.python.org/file27079/assertRaises.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: Sounds like a reasonable suggestion. However, the patch is not valid for 2.7, since there exceptions can be old style classes. -- nosy: +r.david.murray stage: - patch review versions: -Python 2.6, Python 2.7, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: I put some review comments in rietveld (you should have gotten an email). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Daniel Wagner-Hall added the comment: I seem to be getting exceptions why trying to upload a new patch to rietveld, either by the web interface (in several browsers), or by upload.py - attaching new patchset here Also, if I wanted to backport to 2.7 including an isinstance(e, types.ClassType) check, how would I go about doing so? Thanks for the quick review! -- Added file: http://bugs.python.org/file27080/assertRaises.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: Uploading the new patch here is the correct procedure. It will automatically be uploaded to rietveld as well. If by how you mean how to submit a backport, just create a patch against 2.7 tip and upload it separately. Revised patch looks good. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Changes by R. David Murray rdmur...@bitdance.com: -- nosy: +michael.foord ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
Daniel Wagner-Hall added the comment: Added patch for 2.7. I'll sign the contributor form just as soon as I can get to a printer. Thanks for taking me through my first contribution. -- versions: +Python 2.7 Added file: http://bugs.python.org/file27081/issue15836-2.7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15836] unittest assertRaises should verify excClass is actually a BaseException class
R. David Murray added the comment: You are welcome, and thanks for your contribution. -- components: +Library (Lib) -Tests ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15836 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
