Code

Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#10314 closed (fixed)

TestCase assert methods do not accept a msg parameter

Reported by: wwinham Owned by: nobody
Component: Testing framework Version: 1.0
Severity: Keywords:
Cc: christian.oudard@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

django.test.TestCase assert methods, such as assertContains, do not accept a failure message as their final parameter, like the underlying unittest methods do.

Attachments (5)

assert_msg_patch (4.4 KB) - added by christian.oudard@… 5 years ago.
patch2 (10.8 KB) - added by christian.oudard@… 5 years ago.
different way of doing it, without the method decorators
patch_better_unittests (13.5 KB) - added by christian.oudard@… 5 years ago.
better unittests vs. patch2
msg_prefix_patch (19.2 KB) - added by christian.oudard@… 5 years ago.
Prefixes error messages with custom message instead of replacing it
patch_with_docs (22.0 KB) - added by christian.oudard@… 5 years ago.
Added documentation for msg_prefix

Download all attachments as: .zip

Change History (16)

comment:1 follow-up: Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

No reason we shouldn't keep the same API.

comment:2 in reply to: ↑ 1 Changed 5 years ago by sebleier

  • Resolution set to invalid
  • Status changed from new to closed

Replying to Alex:

No reason we shouldn't keep the same API.

Django's test cases have the possibility of having more than one Exception being raised so it doesn't make sense for a single message to characterize the failure of the test.

Changed 5 years ago by christian.oudard@…

comment:3 Changed 5 years ago by christian.oudard@…

  • Resolution invalid deleted
  • Status changed from closed to reopened

Attached a patch which adds msg parameters to all django test case assert* functions.

I didn't understand at first what you meant by having more than one exception raised, but now that I got into the code, I do. There are multiple ways that each assert method can fail. For example, in assertContains, it could have the wrong status code, not contain the text, or not contain the text the correct number of times. Each of these three outcomes has a different default message.

However, I believe that there are cases where each of the default messages is inadequate. Take for example, a test that looks through pages for invalid template variables:

class TemplateTestCase(ClientTestCase):
    def testTemplateError(self):
        urls = [
            '/',
            '/home/',
            '/admin/',
            # etcetera ...
        ]
        for url in urls:
            response = self.client.get(url, follow=True)
            self.assertNotContains(
                response,
                settings.TEMPLATE_STRING_IF_INVALID,
                msg='Found an invalid variable in url %s' % url)

Without the message parameter, it would require print statements to determine which url actually failed.

Changed 5 years ago by christian.oudard@…

different way of doing it, without the method decorators

comment:4 Changed 5 years ago by christian.oudard@…

  • Has patch set

comment:5 Changed 5 years ago by russellm

  • Resolution set to wontfix
  • Status changed from reopened to closed

As sebleier noted, this is an ill-posed request. Assertions like assertFormError raised many different error messages, so a simple one-message parameterization will be insufficient.

If there is a problem with a specific error message (as is suggested by comment 3), the the solution is to fix that error message. Suggestions welcome.

comment:6 Changed 5 years ago by christian.oudard@…

How would you suggest handling use cases such as the above code snippet without a msg parameter?

comment:7 Changed 5 years ago by russellm

If you want to discuss a wontfix, the right place is django-dev.

Changed 5 years ago by christian.oudard@…

better unittests vs. patch2

Changed 5 years ago by christian.oudard@…

Prefixes error messages with custom message instead of replacing it

comment:8 Changed 5 years ago by russellm

  • Needs documentation set
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Reopening following discussion on django-dev that resolved to use the extra argument as a prefix to the internally generated error message of the assertions.

comment:9 Changed 5 years ago by russellm

  • milestone set to 1.2

Marking as v1.2 so I remember to check this in.

Changed 5 years ago by christian.oudard@…

Added documentation for msg_prefix

comment:10 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [12273]) Fixed #10314 -- Added a message prefix argument to Django's test assertions. Thanks to Wes Winham for the original suggestion, and Chistian Oudard for the patch.

comment:11 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.