Opened 10 years ago

Closed 10 years ago

#7978 closed (fixed)

Contrib.Sites doctest assumes existence of current site

Reported by: evan_schulz Owned by: nobody
Component: Testing framework Version: master
Severity: Keywords: sites doctest
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


django.contrib.sites.tests contains the following test checking that the get_current method throws an exception if the current record doesn't exist:

>>> s = Site.objects.get_current()
>>> s.delete()
>>> Site.objects.get_current()
Traceback (most recent call last):
DoesNotExist: Site matching query does not exist.

The problem is that the default Site record (added when the table is created) is deleted by this doctest. This creates an inconsistency when running "./ test" as any test cases run before the contrib.sites test case will have a valid current_site() while those that run after won't. This can be even more confusing for someone creating test cases as running tests for an individual app ("./ test myapp") may work fine while running test cases for all apps could fail (once this sites doctest gets thrown into the mix).

Proposal: Restore the default Site record at the end of the sites doctest so that other test cases can assume the record will be there (as the sites, auth, and possibly other contrib tests already do).

Attachments (1)

sites_test.diff (652 bytes) - added by evan_schulz 10 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 10 years ago by Russell Keith-Magee

Resolution: wontfix
Status: newclosed

It's not the responsibility of doctests to make sure that they have no net impact. If a test requires a site instance to operate cleanly, then that test case should ensure that the site instance is available. Test should not assume that other tests have left the database in a particular state. This means that each test should manually create the object it requires, or invoke the flush command to ensure that initial data exists as expected.

If you can point at a specific test case in the Django suite that is affected by the non-existence of Site, reopen this ticket with the details of the affected test. If the failure is with your own test, you need to update the test.

Changed 10 years ago by evan_schulz

Attachment: sites_test.diff added

comment:2 Changed 10 years ago by evan_schulz

Has patch: set
Resolution: wontfix
Status: closedreopened
Summary: Contrib.Sites doctest deletes current siteContrib.Sites doctest assumes existence of current site

Fair enough -- since the sites doctest currently requires a site instance to operate without ensuring that the initial data exists I've attached a patch updating the test

comment:3 Changed 10 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [8160]) Fixed #7978 -- Modified the contrib.sites tests to guarantee the initial conditions of the test match meet the requirements of the test. Thanks to Evan Schulz for the report and fix.

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