Opened 10 years ago
Closed 11 months ago
#24721 closed New feature (duplicate)
Add "test extensions" (e.g. something similar to mail.outbox for messages)
Reported by: | Marc Tamlyn | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | alasdair@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
TL;DR
- Testing messages is tricky, and depends on the choice of storage used
- Testing email has been abstracted nicely, let's copy that
- Doing so from a contrib app is hard, let's leverage app configs as a place for hooks for contrib apps to inject their own testing needs into Django's normal test environment
The long version
When testing emails with Django, it automatically changes your email backend to a clever in memory one, and exposes mail.outbox
which will contain any emails sent during that test and tidy itself up afterwards for you.
With messages we have no such machinery. If you want to check a message has been sent on a GET this is quite straightforwards as you can simply check response.context['messages']
for it. However with POST requests it is rather more complex, especially if you use assertRedirects
in it's normal form - the check of the destination of the redirect will remove the message!
As an example, consider this test, which is based on an existing test (messages.tests.test_mixins.SuccessMessageMixinTests
):
from django.core.urlresolvers import reverse from django.test import TestCase, override_settings @override_settings(ROOT_URLCONF='messages_tests.urls') class TestRedirectionMessage(TestCase): def test_simple(self): author = {'name': 'John Doe', 'slug': 'success-msg'} add_url = reverse('add_success_msg') response = self.client.post(add_url, author) # response.context['messages'] does not exist as response is a 302 self.assertRedirects(response, reverse('show_message')) # Message is now gone, even if I were to make another GET request.
In this case, the original test inspects response.cookies
for the 302 response, which is fairly easy, but it's more complex with other message storages. If you use the default FallbackStorage
, you would have to change your test if the message got too long to inspect the session storage instead of the cookie...
Here's the version of that test I would like:
def test_simple(self): author = {'name': 'John Doe', 'slug': 'success-msg'} add_url = reverse('add_success_msg') response = self.client.post(add_url, author) self.assertRedirects(response, reverse('show_message')) self.assertEqual(len(messages.outbox), 1) self.assertEqual(messages.outbox[0].message, 'John Doe was created successfully') self.assertEqual(messages.outbox[0].level, messages.SUCCESS)
The use of the name outbox
here is somewhat strange I admit, but as yet I haven't found an alternative.
The actual implementation detail is fairly straightforwards in some sense - create a new InMemoryStorage
with the same basic infrastructure as the corresponding email backend. The difficult part is that due to the fact this is a contrib app. The engineering to make the email backend work takes place in two distinct locations - first in the setup_test_environment
to initialise it, and then in TestCase._pre_setup
to reset before each test.
The best proposal I have for designing this is to add some optional methods to the AppConfig
class which are hooks called by both of these methods. This would also be extremely useful for other packages which interact with external resources and/or have alternate backends. For example, django-redis could use these hooks to switch to another redis database and automatically clear it before each test (like we do with other data stores), or django-celery could automatically switch to using an in-memory broker and provide a good api for checking that the task has been queued, then flushing the queue later if desired, if not then reset the queue after each test. This is likely to be more expressive and performant than CELERY_ALWAYS_EAGER = True
.
Backwards compatibility with existing tests which expect the request to have been modified with messages is an issue, I suspect this may need to be phased in, or set as non default. This could be done via an alternative app config if this is the route we choose to go.
Change History (10)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
An overridden test runner works very well for changing the test environment (changing settings for example), but it doesn't allow for wrapping tests. I personally am not a huge fan of the following pattern:
MyTestMixins(object): def _pre_setup(self): redis.clear() queue.clear() messages.outbox = [] ... MyTestCase(MyTestMixins, TestCase) MyTransactionTestCase(MyTestMixins, TestCase) MyLiveServerTestCase(MyTestMixins, MyLiveServerTestCase) ...
comment:3 by , 10 years ago
Yeah, I use pytest-django
and pytest fixtures for that, which I think is a much better pattern, but not one we can enforce for Django. So it looks like this is another case where we'll end up reinventing a Django-specific solution to a problem that's already better-solved elsewhere, because we don't want to take on the better solution as a dependency :/
comment:4 by , 10 years ago
A more explicit design possibility which doesn't require so much end user code and provides a more explicit (and more optional) hook:
# settings.py TEST_RUNNER = 'myproject.test_runner.TestRunner' # test_runner.py class TestRunner(DiscoverRunner): test_extensions = [messages.test.MessagesExtension, django_redis.test.TestExtension]
Each extension class would have hooks for test environment setup and teardown, before/after each test case and individual test.
At the moment I find most third party apps are very bad at making it easy to use them in a suitable manner for tests - I hope by introducing an official API and making it easy for users they will consider more providing utilities for testing.
I'd love to find an elegant way for these extensions to provide custom assertion methods as well, for example self.assertMessageSent('you failed', level=messages.ERROR)
, but this would require something more magical involving getattrs, deliberate monkeypatching on of methods or something equally ugly.
At this point it does start looking a lot like stuff pytest has done better. Personally I've not had the chance to use it a huge amount yet, but it does look good. I think that pytest is now big enough and established enough that we could consider adding a django.test.pytest
module with utilities along the lines of those in pytest-django
.
Mind you, if we are talking about magical code in test utilities, pytest takes it to a whole new level ;)
comment:5 by , 10 years ago
I like the "test extensions" idea much better than automatically-run hooks on AppConfig
. It's still a bit of an "NIH" partial re-implementation of pytest fixtures, but at least it is fully explicit (and these extension classes could be structured in such a way that they could be easily reused as a pytest fixture, which would be awesome). I wouldn't even mind if said "test extensions" were configured via their own setting rather than as an attribute of a custom TestRunner
, to make them easier to use (I'm not allergic to new settings).
Re the idea of django.test.pytest
, I think there's some merit to the idea but it's getting OT for this ticket, so we can discuss it elsewhere :-)
And it's not "magic" per se that concerns me here - it's implicitly and automatically monkeypatching things only when under test. It's important that developers be aware of how their system under test differs from their system not under test, which is why I don't think such changes should be made automatically, in most cases.
comment:6 by , 10 years ago
Cc: | added |
---|
comment:7 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:9 by , 9 years ago
Component: | contrib.messages → Testing framework |
---|---|
Has patch: | set |
Needs documentation: | set |
Patch needs improvement: | set |
Summary: | Introduce something similar to mail.outbox for messages → Add "test extensions" (e.g. something similar to mail.outbox for messages) |
comment:10 by , 11 months ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Duplicate of #34730, fixed by adding assertMessages()
.
I think an in-memory backend for
contrib.messages
, for testing purposes, is a great idea.I'm less sold on doing more automatic monkeypatching for tests, or adding a hook for third-party apps to automatically monkeypatch themselves when under test. I'd really like to see such changes be explicit somehow, rather than automatic. Personally I just use a test-specific settings file with a few settings overrides, but there may be other options.