Opened 8 years ago

Closed 8 years ago

#28224 closed Cleanup/optimization (fixed)

Test for SuspiciousOperation subclasses rather than SuspiciousOperation in Django's tests

Reported by: Mads Jensen Owned by: Rajesh Veeranki
Component: Core (Other) Version: dev
Severity: Normal Keywords: exception classes
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

tests/requests/tests.py contains some less specific tests. Actually, it's DisallowedHost that's being raised instead for all of these cases. Isn't it better in general to test for the actual exception class? Probably there are other cases of tests like this that could be made more specific.

with self.assertRaises(SuspiciousOperation):

Change History (5)

comment:1 by Tim Graham, 8 years ago

Summary: Test suite exception catching is not always specific → Use more specific exception testing in Django's tests
Triage Stage: Unreviewed → Accepted

comment:2 by Rajesh Veeranki, 8 years ago

Owner: changed from nobody to Rajesh Veeranki
Status: new → assigned

comment:3 by Rajesh Veeranki, 8 years ago

Has patch: set

Please review the PR here: ​https://github.com/django/django/pull/8545
I have searched the tests for any instances of SuspiciousOperation and replaced with appropriate derived exception

comment:4 by Tim Graham, 8 years ago

Summary: Use more specific exception testing in Django's tests → Test for SuspiciousOperation subclasses rather than SuspiciousOperation in Django's tests
Triage Stage: Accepted → Ready for checkin

comment:5 by Tim Graham <timograham@…>, 8 years ago

Resolution: → fixed
Status: assigned → closed

In 67e1afb4:

Fixed #28224 -- Tested for SuspiciousOperation subclasses in Django's tests.

Note: See TracTickets for help on using tickets.
Back to Top