#14319 closed New feature (wontfix)
Add signals test_setup and test_teardown to Django test suite runner Options
Reported by: | Jim Dalton | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | signals |
Cc: | adam@…, Jim Dalton | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
I would like to submit a proposal (with patch) for two new signals to add to the testing framework: a test_setup signal, to be emitted during global test setup, and a test_teardown signal, to be emitted during global test teardown.
Presently, there is no way to modify or add to the global setup or teardown behavior that runs with the built-in test suite runner. This patch provides a hook that applications can use to do that. Each signal is emitted once during a test run.
The proposal was discussed briefly on the Django developer's list here:
http://groups.google.com/group/django-developers/browse_thread/thread/f45c7302634ed82d
Here are a few notes about the proposed changes to source code:
- There are really only a handful of lines of code required to make this change.
- The most significant change is that the ordering of actions in the
run_tests()
method ofDjangoTestSuiteRunner
has been adjusted. In order for applications to be able to setup connect() calls to the signals, the application code itself has to be imported. This happens for the first time inbuild_suite()
, so I swapped the order of the call tobuild_suite()
and the call tosetup_test_environment()
. This appears to have no impact, but it's conceivable that some issues could arise if e.g. an application developer did something during import that they expected to be disabled first during testing. At present, the main thing Django does in thesetup_test_enivronment()
call is to disable the email backend, so unless a developer is sending emails in the body of a module or something it shouldn't present a problem. - The second most significant change is that the
django.test.utils
functionssetup_test_environment()
andtear_down_environment()
calls are executed by connecting to the signal rather than being called directly from the methods of the test runner. This serves as a way to test that the signals are being fired, as Django tests will fail if those two functions do not execute. (Thanks to Russell Keith-Magee for this suggestion). - I have included documentation for these two new signals on the Django signals documentation page.
Here are a few arguments in favor of this proposal that I posted to the Django developers discussions:
- Requiring a custom test runner to implement this behavior makes it (nearly) impossible for reusable applications to modify global setup and teardown behavior, since it would become the responsibility of the project itself to specify the custom test runner.
- The current setup gives the Django core "privileged" access to disable certain features during testing, it would seem that application should be given the capability as well.
- Signals are non obtrusive...if they are not used they don't really harm anything.
- None of the proposed changes would impact production code, since they are all restricted to the test suite. In fact the patch is really only about a few lines of additional code.
There maybe other arguments in favor of or against as well, please feel free to share them if so.
Attachments (3)
Change History (16)
by , 14 years ago
Attachment: | new_test_signals_patch.diff added |
---|
comment:1 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
Keywords: | signals added |
---|---|
Severity: | → Normal |
Type: | → New feature |
comment:3 by , 14 years ago
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
new_test_signals_patch.diff fails to apply cleanly on to trunk
by , 14 years ago
Attachment: | new_test_signals_patch_2.diff added |
---|
Updated patch, applies against trunk as of r16181
comment:4 by , 14 years ago
Cc: | added |
---|
I'd like to have this feature as well. I've attached an updated patch that applies against trunk. (Only trivial changes from the original patch.)
The documentation with this patch still refers to its becoming available in Django 1.3. I assume that should be updated, but I'm not sure what to update it to?
comment:5 by , 14 years ago
Cc: | added |
---|---|
milestone: | 1.3 → 1.4 |
Patch needs improvement: | unset |
Thanks @aseering. Looks like there were a few additional issues applying your patch against trunk, so I cleaned those up. I also bumped the version to 1.4 in the docs.
This now applies cleanly against r16277 and all tests pass. Hopefully this can now be marked as ready for checkin by a reviewer.
by , 14 years ago
Attachment: | new_test_signals_patch_3.diff added |
---|
follow-up: 7 comment:6 by , 13 years ago
Patch needs improvement: | set |
---|---|
UI/UX: | unset |
It is unclear why the patch is removing the global setup and tear down calls setup_test_environment and teardown_test_environment
comment:7 by , 13 years ago
Patch needs improvement: | unset |
---|
Replying to ptone:
It is unclear why the patch is removing the global setup and tear down calls setup_test_environment and teardown_test_environment
I mentioned this up in the ticket description:
"The second most significant change is that the django.test.utils functions setup_test_environment() and tear_down_environment() calls are executed by connecting to the signal rather than being called directly from the methods of the test runner. This serves as a way to test that the signals are being fired, as Django tests will fail if those two functions do not execute. (Thanks to Russell Keith-Magee for this suggestion)."
Basically those functions are hooked to the new global setup/teardown signals here rather than being called directly. Makes sense?
comment:9 by , 13 years ago
Patch needs improvement: | set |
---|
OK, I see - there is an import that needs to be deleted, I think that might have been what was throwing me off.
Also, even though the preceding item mentions it, I think it is redundant to say that this signal does not fire during normal operation in each test description, since that caveat is introduced in the test signals section.
Another possible heads up that came from IRC, is that there is some discussion about abandoning the Django custom test harness in place of a standardized one based on nose or test.py, and so there is some concern about adding features to the current harness
comment:10 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
After a discussion with Carl yesterday I'm marking this as wontfix. This idea continues a fundamentally bad trend in Django: treating it as some sort of application runner, rather than a library. To that end, our long running goal should be to kill our test harness, and recreate the database setup stuff as plugins for unittest2 (which means it's then runnable by other test framworks like nose or py.test), once we're firmly in that territory users will have the same things available to them: they can write unittest plugins to do their own test suite setup.
comment:11 by , 13 years ago
What you're saying sounds reasonable but it's hard for me to really understand the context in which this decision has been made. Is there a ticket or discussion that relates to this idea of moving the Django test suite toward a "plugin" approach? I googled plugins for unittest2 but I couldn't really find anything about it either.
I'd be happy to assist in any efforts that are being made to move the testing suite in Django in the direction you are suggesting, but I'm concerned that we're killing a useful improvement for the sake of future ambitions that might never materialize.
follow-up: 13 comment:12 by , 11 years ago
Has any progress been made towards recreating the database setup stuff as plugins for unittest2? Or is there a ticket to track progress on that front?
I know that Django 1.6 shipped with a new test runner allowing improved and more consistent test discovery, but that doesn't seem to be a step in the direction of making Django less of an application runner and more of a library.
In fact, Django 1.7 will ship with the app-loading refactor which promises "applications can run code at startup, before Django does anything else, with the read() method of their configuration".
I could be wrong, but doesn't that seem to indicate that, bad trend or not, treating Django as some sort of application runner is here to stay. If so, and especially if there are no concrete plans to remove the test runner and replace it with unittest2 plugins for database setup as described above, it would be good to have startup hooks in the test runner as provided by this ticket.
For my own use case, I have a custom management command that performs some initial application setup and I need to run before any tests, and would like to avoid running it in every TestCase.setUp()
method only to be rolled back and re-run for the next test.
comment:13 by , 11 years ago
Replying to mrmachine:
Has any progress been made towards recreating the database setup stuff as plugins for unittest2? Or is there a ticket to track progress on that front?
Well, I'm not even sure what you mean by this, so it's probably safe to say there isn't a ticket.
I know that Django 1.6 shipped with a new test runner allowing improved and more consistent test discovery, but that doesn't seem to be a step in the direction of making Django less of an application runner and more of a library.
In fact, Django 1.7 will ship with the app-loading refactor which promises "applications can run code at startup, before Django does anything else, with the read() method of their configuration".
I could be wrong, but doesn't that seem to indicate that, bad trend or not, treating Django as some sort of application runner is here to stay. If so, and especially if there are no concrete plans to remove the test runner and replace it with unittest2 plugins for database setup as described above, it would be good to have startup hooks in the test runner as provided by this ticket.
I'm not sure how you've come to this conclusion. Django isn't transforming into a "runner". It's providing a library-level mechanism that apps can tie into so that they can register "once at startup" logic. There is no master "Django" process; it's still up to the wsgi container (or whatever is running the Django stack) to configure the Django library; making sure all the apps are configured are now part of that remit.
For my own use case, I have a custom management command that performs some initial application setup and I need to run before any tests, and would like to avoid running it in every
TestCase.setUp()
method only to be rolled back and re-run for the next test.
Sure - I can see the value in this; but I'm not sure why it isn't possible with an extension of the test runner. There might be room to make the interface cleaner, but AFAICT it's *possible* right now.
Patch to add test_setup and test_teardown signals to Django test framework