classification
Title: unittest.assert*Regex functions should verify that expected_regex has a valid type
Type: enhancement Stage: resolved
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ethan.furman, ezio.melotti, kamie, michael.foord, pitrou, python-dev, r.david.murray, the.mulhern
Priority: normal Keywords: easy, patch

Created on 2014-01-06 14:28 by the.mulhern, last changed 2014-03-25 19:35 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
validate_regex.patch kamie, 2014-03-11 01:28 patch to check expected_regex review
validate_regex_improved.patch kamie, 2014-03-11 23:51 patch to check expected_regex improved review
Messages (14)
msg207436 - (view) Author: the mulhern (the.mulhern) Date: 2014-01-06 14:28
A normal thing for a developer to do is to convert a use of an assert* function to a use of an assert*Regex function and foolishly forget to actually specify the expected regular expression.

If they do this, the test will always pass because the callable expression will be in the place of the expected regular expression and the callable expression will, therefore, be None. It would be nice if in the constructor in AssertRaisesBaseContext (for 3.5) not only was the expected regex converted to a regex (if actually a string or bytes) but if it's not if it were checked if it were a valid regular expression object and an exception raised if this is not the case.

In the current version of AssertRaisesBaseContext.handle the comments say that if the callable object is None then the function is being used as a context manager. Not always the case, alas, perhaps the developer just forgot to add that necessary regular expression before the callable object.
msg210804 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2014-02-10 07:52
The two signatures of assertRaisesRegex are:
assertRaisesRegex(exception, regex, callable, *args, **kwds)
assertRaisesRegex(exception, regex, msg=None)

IIUC what you are saying is that if you forget the regex and call assertRaisesRegex(exception, callable) the test will pass.

I think it would be ok to add a check to see if the second argument is a callable or None (or maybe check if it's a string or regex object?), and give an appropriate error message if it's not.
msg210828 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-02-10 12:03
Agreed.
msg211417 - (view) Author: the mulhern (the.mulhern) Date: 2014-02-17 13:47
Yes. I'ld check if it was a string or a regex object...there is already code that converts the string to a regular expression in there.
msg212923 - (view) Author: Kamilla (kamie) * Date: 2014-03-08 04:45
Just to be sure, the check must be implemented inside the assertRaisesRegex method, right?
msg212944 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-03-08 22:10
That's correct.
msg212961 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-03-09 15:39
A simple way of checking is to actually re.compile(regex), I think. It should automatically raise TypeError on invalid input.
msg212963 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-09 16:06
Heh.  If unittest had used duck typing instead of isinstance for its string-to-regex conversion check in the first place, this issue wouldn't even have come up.
msg213110 - (view) Author: Kamilla (kamie) * Date: 2014-03-11 01:28
I've implemented a piece of code to check if the expected_regex is a string or regex object. If it's not it raises a TypeError exception. The check is inside the __init__ method of the _AssertRaisesBaseContext class so it will always check the expected_regex in all methods that use the _AssertRaises.

I've added tests too. They are verifying the assertRaisesRegex and the assertWarnsRegex methods.
msg213186 - (view) Author: Kamilla (kamie) * Date: 2014-03-11 23:51
Applying changes as suggested by R. David Murray in the Core-mentorship e-mail list.

Instead of doing the if tests I've replaced the existing

   if isinstance(expected_regex, (bytes, str)):

by

   if expected_regex is not None:


And also made a change in one of the tests because I figure out it was wrong.
msg213254 - (view) Author: the mulhern (the.mulhern) Date: 2014-03-12 12:16
Thanks, I'ld definitely be _much_ happier w/ a TypeError than with silent success.
msg214628 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-23 19:47
New changeset ec556e45641a by R David Murray in branch 'default':
#20145: assert[Raises|Warns]Regex now raise TypeError on bad regex.
http://hg.python.org/cpython/rev/ec556e45641a
msg214629 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-03-23 19:50
Thanks, Kammie.  I removed the extra whitespace from your fix and simplified the tests a bit.
msg214846 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-25 19:35
New changeset 3f8b801e7e76 by R David Murray in branch '2.7':
backport: #20145: assertRaisesRegexp now raises a TypeError on bad regex.
http://hg.python.org/cpython/rev/3f8b801e7e76

New changeset 32407a677215 by R David Murray in branch '3.4':
backport: #20145: assert[Raises|Warns]Regex now raise TypeError on bad regex.
http://hg.python.org/cpython/rev/32407a677215

New changeset 8f72f8359987 by R David Murray in branch 'default':
Merge #20145 backport: delete whatsnew entry.
http://hg.python.org/cpython/rev/8f72f8359987
History
Date User Action Args
2014-03-25 19:35:36python-devsetmessages: + msg214846
2014-03-23 19:50:08r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg214629

stage: test needed -> resolved
2014-03-23 19:47:27python-devsetnosy: + python-dev
messages: + msg214628
2014-03-20 00:54:59ethan.furmansetnosy: + ethan.furman
2014-03-12 12:16:44the.mulhernsetmessages: + msg213254
2014-03-11 23:51:24kamiesetfiles: + validate_regex_improved.patch

messages: + msg213186
2014-03-11 01:28:32kamiesetfiles: + validate_regex.patch
keywords: + patch
messages: + msg213110
2014-03-09 16:06:40r.david.murraysetnosy: + r.david.murray
messages: + msg212963
2014-03-09 15:39:53pitrousetnosy: + pitrou
messages: + msg212961
2014-03-08 22:10:42michael.foordsetmessages: + msg212944
2014-03-08 04:45:09kamiesetnosy: + kamie
messages: + msg212923
2014-02-17 13:47:49the.mulhernsetmessages: + msg211417
2014-02-10 12:03:05michael.foordsetmessages: + msg210828
2014-02-10 07:52:29ezio.melottisetkeywords: + easy

messages: + msg210804
2014-01-11 08:23:10terry.reedysetnosy: + ezio.melotti, michael.foord
stage: test needed
type: behavior -> enhancement

versions: - Python 3.1, Python 2.7, Python 3.2, Python 3.3, Python 3.4
2014-01-06 14:28:17the.mulherncreate