Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15111 closed Uncategorized (fixed)

manage.py test fails if sites app not installed

Reported by: wkornewald Owned by: nobody
Component: Contrib apps Version: master
Severity: Normal Keywords:
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

The unit tests of several contrib apps depend on the sites app even though the respective contrib app already falls back to RequestSite. The attached patch fixes the failing tests (at least the ones I've stumbled upon). Not sure how this patch relates to the proposed changes in #15089, though.

Attachments (1)

site_not_installed.diff (6.7 KB) - added by wkornewald 3 years ago.
patch against r15239

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by wkornewald

patch against r15239

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

Marking RFC because the patch is pretty much ready to use.

However, I have a suspicion that the very last delta is wrong (or at least requires additional thought). The call to get_current() is intended to populate a cache; it shouldn't be possible to remove that line without consequences.

comment:2 Changed 3 years ago by wkornewald

Maybe if the purpose of the last line is to check if the exception is raised even with the Site cache filled we might need a small extra change. But then I'd rather extend get_current(), so it's guaranteed to raise Site.DoesNotExist if Site._meta.installed is False and have a unit test in the sites app for that.

OTOH, I'd question the whole purpose of testing against a filled cache with Site._meta.installed set to False via a hack because then the unit test would verify if everything works correctly even if people use ugly hacks in their code (i.e., manipulating Site._meta.installed in production code).

comment:3 Changed 3 years ago by wkornewald

  • Cc wkornewald@… added

comment:4 Changed 3 years ago by russellm

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

In [15418]:

Fixed #15111 -- Ensured that the auth, contenttypes and sitemaps tests will run when the sites app isn't installed. Thanks to Waldemar Kornewald for the report and draft patch.

comment:5 Changed 3 years ago by russellm

In [15419]:

[1.2.X] Fixed #15111 -- Ensured that the auth, contenttypes and sitemaps tests will run when the sites app isn't installed. Thanks to Waldemar Kornewald for the report and draft patch.

Backport of r15418 from trunk.

comment:6 Changed 3 years ago by ramiro

In [15429]:

[1.2.X] Fixed small problem in backport of [15418] tests changes to 1.2.X ([15419]). Refs #15111.

comment:7 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:8 Changed 3 years ago by wkornewald

  • Cc wkornewald@… removed
  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

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.