Skip to content

WIP #21230: Remove some direct settings manipulations in tests #1739

wants to merge 14 commits into from

3 participants

Bouke commented Oct 14, 2013

See ticket 21230. The ticket is marked as 'easy pickings', however there is quite some work to be done and not everything is straight-forward. So I started off with some low hanging fruit. This doesn't close the ticket, as there are quite a few remaining places where settings are manipulated.

There might be some problem with code style, especially in places of multiline decorator calls. Please advice on how the patch should be improved.

This is the progress so far on rewriting the occurrences of direct settings manipulations:

  • admin_views/
  • comment_tests/tests/
  • generic_inline_admin/
  • i18n/
  • managers_regress/
  • middleware_exceptions/
  • modeladmin/
  • proxy_models/
  • select_for_update/
  • serializers/
  • settings_tests/
  • staticfiles_tests/
  • template_tests/
  • template_tests/
  • template_tests/
  • test_client/
  • urlpatterns_reverse/
Django member

Looks ok to me. I've not run the tests yet to check they pass. Did you want to do some more of the files or shall I tidy up?

(I'm at the pycon ie sprint btw)

Bouke commented Oct 14, 2013

Well I've had my share for today, but could put in some work tomorrow.

(Have fun at pycon ie!)

Bouke commented Oct 14, 2013
  • The tests in serializers do not inherit from Django's TestCase and cannot be decorated with override_settings.
  • The tests in staticfiles_tests are complex and should be treated with extra care.
  • I've modified the tests in select_for_update, but those require a compatible DB, which I don't have available at this moment.
Bouke commented Oct 15, 2013

I have got (approx 80%) of all direct settings manipulations removed. The remaining (see the checklist above) are more elaborate manipulations, which I'd rather leave untouched. The staticfiles tests being the most sophisticated with stacked class inheritance, which probably need more extensive rewriting.

Django member

merged in 3565efa - thanks!

@timgraham timgraham closed this Oct 21, 2013
@Bouke Bouke deleted the Bouke:ticket_21230 branch Oct 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.