Code

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#5296 closed (wontfix)

[patch] Additions to TestCase for easy view testing

Reported by: Chris H. <chris@…> Owned by: adrian
Component: Testing framework Version: master
Severity: Keywords: view client unit testing
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

At work we frequently have testing idioms likes this:
def test_x(self):

...do database stuff...
response = self.client.get('/some/url/')
check the response status code
check the response template
check the response content
check the response content_type

I'm attaching a patch, with tests, that adds a "check_view" method to TestCase that wraps up the common idiom of checking the status code, the template, the content and the content_type.

In my mind it's somewhat analogous to render_to_response -- it's a shortcut for a common idiom, and it doesn't prevent you from doing more complex/interesting testing with the other assertX methods.

Attachments (1)

easy_view_testing.diff (6.1 KB) - added by Chris H. <chris@…> 7 years ago.
Additions to TestCase and tests to exercise those additions.

Download all attachments as: .zip

Change History (4)

Changed 7 years ago by Chris H. <chris@…>

Additions to TestCase and tests to exercise those additions.

comment:1 Changed 7 years ago by russellm

  • Needs documentation set
  • Needs tests unset
  • Patch needs improvement unset

I see the point you're making regarding render_to_response, but I'm not entirely convinced. Your own test example demonstrates the problem that I have with this idea. Even in your test case, you can't express the test as a single statement - for clarity, you break the test into predefined constants and then the function call.

render_to_response works because every page render requires a template name and a data dictionary, with a base context being provided as an optional extra. There is really only 1 option (strictly, the data dictionary is optional too), and the argument order makes a good deal of sense, so there isn't any confusion about where the optional argument goes.

In a unified test method such as yours, all the arguments (except the response) are optional - this means that every time you call the method, you need to either rely upon an argument order (and it isn't obvious to me what the 'correct' argument order should be), or you need to specify kwarg names, which detracts from your 'simple' argument. Ultimately, all you end up with is one call that wraps calls to all the other assert statements, with optional arguments that allow each test to be ignored. I'm not sure I see the value in such a wrapper.

For me, testing is all about the clear statement of atomic tests. Every view is slightly different, so every view will require a slightly different set of atomic tests. The assert syntax isn't _that_ onerous as is, and each assert is atomic. It's very clear what is being tested, when, and in what order.

comment:2 Changed 7 years ago by Simon G. <dev@…>

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

I'm marking this as wontfix. Chris H. - please re-open this if you disagree/respond to Russell's comments above.

comment:3 Changed 7 years ago by ChrisH

Simon, I think it's fine to leave it as wontfix... Russell's comments make sense...

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.