[Django] #25948: documentation doesn't explain why assertRaisesMessage is needed
Django
--------------------------------------+--------------------
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
Apparently there is a good reason for `assertRaisesMessage` to exists, but
documentation doesn't say a word if it's preferred over
`assertRaisesRegex`. If it's really preferred, it's usage should be
explicitly encouraged.
--
Ticket URL: <https://code.djangoproject.com/ticket/25948>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sir-sigurd | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
It was added in #14503. Documentation says, "Similar to unittest’s
assertRaisesRegex() with the difference that expected_message isn’t a
regular expression." I guess with `assertRaisesRegex` you will have to
call `re.escape(expected_message)` to ensure the message isn't treated as
a regular expression. Should that point be noted? Another advantage from
my perspective is that `assertRaisesMessage` won't need to change when
dropping Python 2, unlike `six.assertRaisesRegex` (`six` -> `self` and
removing `self` argument).
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:1>
Django
Reporter: sir-sigurd | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sir-sigurd):
I think that documentation should say that behavior of
`assertRaisesMessage` is more straightforward and that `assertRaisesRegex`
should be used only if regex is really needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:2>
Django
Reporter: sir-sigurd | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Is this explicit enough?
{{{ #!diff
diff --git a/docs/topics/testing/tools.txt b/docs/topics/testing/tools.txt
index c41e35d..4491878 100644
--- a/docs/topics/testing/tools.txt
+++ b/docs/topics/testing/tools.txt
@@ -1230,10 +1230,11 @@ your test suite.
.. method:: SimpleTestCase.assertRaisesMessage(expected_exception,
expected_message, callable_obj=None, *args, **kwargs)
Asserts that execution of callable ``callable_obj`` raised the
- ``expected_exception`` exception and that such exception has an
- ``expected_message`` representation. Any other outcome is reported as
a
- failure. Similar to unittest's
:meth:`~unittest.TestCase.assertRaisesRegex`
- with the difference that ``expected_message`` isn't a regular
expression.
+ ``expected_exception`` exception and that ``expected_message`` is in
the
+ exception's representation. Any other outcome is reported as a
failure.
+ It's a simpler version of :meth:`unittest.TestCase.assertRaisesRegex`
with
+ the difference that ``expected_message`` isn't treated as a regular
+ expression.
.. method:: SimpleTestCase.assertFieldOutput(fieldclass, valid, invalid,
field_args=None, field_kwargs=None, empty_value='')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:3>
Django
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:4>
Django
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sir-sigurd):
Replying to [comment:3 timgraham]:
It's nice adjustment, but I thought that we should document somewhere that
`assertRaisesMessage` is preferred for Django internal usage. Does this
make sense?
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:5>
Django
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
[https://github.com/django/django/pull/5856 PR] as you requested.
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:6>
Django
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"77f50c9cfc8b33bf86394816cde183e656ba28cd" 77f50c9]:
{{{
#!CommitTicketReference repository=""
revision="77f50c9cfc8b33bf86394816cde183e656ba28cd"
Fixed #25948 -- Added guidelines for SimpleTestCase.assertRaisesMessage()
usage.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:7>
Django
Reporter: sir-sigurd | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"c0d8d5f98c7f3484a254bbefbb4527aa0c6791d2" c0d8d5f9]:
{{{
#!CommitTicketReference repository=""
revision="c0d8d5f98c7f3484a254bbefbb4527aa0c6791d2"
[1.9.x] Fixed #25948 -- Added guidelines for
SimpleTestCase.assertRaisesMessage() usage.
Backport of 77f50c9cfc8b33bf86394816cde183e656ba28cd from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25948#comment:8>