Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner
Cool, thanks On Sep 20, 3:30 pm, Carl Meyer wrote: > Thanks for your work on this! The usual Django workflow doesn't > include patches to the mailing list: rather you can go ahead and open > a Trac ticket and attach the patch there (even if you aren't sure of > the approach yet), and reference the ticket number here. Cool, thank you Carl. I'll go ahead and submit it as a ticket and let it take its natural course from there. > So where this would break is if someone is doing a bit too much at > import time of their tests module (like in the class body of a > TestCase subclass): for instance, saving something to the database. I > have to admit (gulp) that I have actually done this before and it > currently works fine, but it's not really good practice and it's > certainly not documented anywhere that you can do that, so personally > I'm not sure it would be a problem to break it. Yeah, that's what I was thinking too. Maybe other people will have examples where it might cause some critical issues, I guess we'll see. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner
Hi Jim, On Sep 19, 1:19 am, "Jim D." wrote: > I found some time this evening to work this out, and have included a > revised patch for this proposal at the end of this message. Thanks for your work on this! The usual Django workflow doesn't include patches to the mailing list: rather you can go ahead and open a Trac ticket and attach the patch there (even if you aren't sure of the approach yet), and reference the ticket number here. > * The addition of this feature required what I imagine to be a > significant change to test suite runner. Namely, I switched the order > of the build_suite() and setup_test_environment() methods in > run_tests(). So where this would break is if someone is doing a bit too much at import time of their tests module (like in the class body of a TestCase subclass): for instance, saving something to the database. I have to admit (gulp) that I have actually done this before and it currently works fine, but it's not really good practice and it's certainly not documented anywhere that you can do that, so personally I'm not sure it would be a problem to break it. Carl -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner
I found some time this evening to work this out, and have included a revised patch for this proposal at the end of this message. I tried to keep changes to an absolute minimum, but here are a few notes about the changes and decisions I did make: * The addition of this feature required what I imagine to be a significant change to test suite runner. Namely, I switched the order of the build_suite() and setup_test_environment() methods in run_tests(). The reason for this has to do with what i suggested in my earlier reply in this thread, namely that there isn't a feasible way to connect to the signals from application code until after applications have been imported. I also wanted to attempt to implement Russ's suggestion of having Django itself use the signals for setup and teardown. The most logical and clean way to do this (and the only way to organize things such that Django could use the code) was to construct the run_test() sequence such that test suites are first built, then setup, then run tests, then teardown. The Django tests suite passed when the order was inverted, but I am quite certain it is a huge assumption on my part that the order can be switched here. Does anybody have any opinions on this, or reasonings as to why the setup_test_environment() stuff would need to be called before build_suite()? I imagine this would be the greatest concern with my proposed patch. * The test utility methods setup_test_environment() and teardown_test_environment() are now connected to the respective signals and are executed when those emit, rather than being called directly from the test suite runner. The **kwargs was added to their signature so that the they could be executed via the signal. * Aside from adding the code to create the signals and to emit them in the test runner, that's all the changes that were necessary to implement this feature. * I have included documentation changes in the patch in what is hopefully a tolerable first stab. I welcome any additional feedback or suggestions. Provided there are no serious objections or deal breakers pointed out in this discussion, I'll submit a ticket for this, which I assume is the best way to move it forward in the official discussion and review process. Here's my revised proposed patch: Index: django/test/simple.py === --- django/test/simple.py (revision 13861) +++ django/test/simple.py (working copy) @@ -7,6 +7,7 @@ from django.test import _doctest as doctest from django.test.utils import setup_test_environment, teardown_test_environment from django.test.testcases import OutputChecker, DocTestRunner, TestCase +from django.test.signals import test_setup, test_teardown # The module name for tests outside models.py TEST_MODULE = 'tests' @@ -230,7 +231,7 @@ self.failfast = failfast def setup_test_environment(self, **kwargs): -setup_test_environment() +test_setup.send(sender=self) settings.DEBUG = False def build_suite(self, test_labels, extra_tests=None, **kwargs): @@ -284,7 +285,7 @@ connection.creation.destroy_test_db(old_name, self.verbosity) def teardown_test_environment(self, **kwargs): -teardown_test_environment() +test_teardown.send(sender=self) def suite_result(self, suite, result, **kwargs): return len(result.failures) + len(result.errors) @@ -308,8 +309,8 @@ Returns the number of tests that failed. """ +suite = self.build_suite(test_labels, extra_tests) self.setup_test_environment() -suite = self.build_suite(test_labels, extra_tests) old_config = self.setup_databases() result = self.run_suite(suite) self.teardown_databases(old_config) Index: django/test/signals.py === --- django/test/signals.py (revision 13861) +++ django/test/signals.py (working copy) @@ -1,3 +1,5 @@ from django.dispatch import Signal template_rendered = Signal(providing_args=["template", "context"]) +test_setup = Signal() +test_teardown = Signal() \ No newline at end of file Index: django/test/utils.py === --- django/test/utils.py(revision 13861) +++ django/test/utils.py(working copy) @@ -52,7 +52,7 @@ return self.nodelist.render(context) -def setup_test_environment(): +def setup_test_environment(**kwargs): """Perform any global pre-test setup. This involves: - Installing the instrumented test renderer @@ -71,8 +71,9 @@ mail.outbox = [] deactivate() +signals.test_setup.connect(setup_test_environment) -def teardown_test_environment(): +def teardown_test_environment(**kwargs): """Perform any global post-test teardown. This involves: - Restoring the original test renderer @@ -89,6 +90,7 @@ del mail.original_email_backend
Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner
Thanks for the suggestions and guidance Russ. I'll work with it a bit more and see if it pans out. My main concern with this working smoothly in the core has to do with when and how application code is loaded in the process of setting up tests. It turns out the earliest we can realistically send the test_setup signal is in the build_suite() method, right after the get_app() / get_apps() functions have executed, i.e. the applications themselves have been loaded. Prior to this any attempts to connect() to the signal won't get picked up, because no applications have been loaded. There's also a related issue, which is that if you run individual tests or sets of tests for an application, it doesn't appear that other applications get loaded. This means that, in a given application, I might add a hook and expect it to run whenever tests for the project are run, but it wouldn't in fact run unless that application was included in the test. So e.g. if I wanted to disable certain behavior whenever tests were run, that might not in fact happen. That said, I guess if another application includes the application where the hooks live, those connect() calls would get set up at the same time. Not sure if what I'm saying is totally clear, but I guess the point is there are a few more complexities here in the setup process than might first meet the eye. Also, given the above, it might not be realistic to configure the existing setup code to run via this signal, since the signal happens so much later in the process. The solution is probably for me to think this through a bit more and see if there isn't a way to send the test_setup signal where one would expect it should be sent (i.e. at the end of the setup_test_environment() method in the suite runner) and at the same time ensure that applications have a way to connect to the signal. I'll give it some thought. Thank you again for the support and suggestions. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner
On Sat, Sep 18, 2010 at 12:49 AM, Jim D. wrote: > I recently asked a question on Django Users related to executing > certain code during the global setup and teardown routines that run in > Django's test runner. In my particular use case, I was looking for a > hook where I could disable some third party API code when tests > execute, in much the same way that Django itself swaps out the email > backend during testing to ensure no emails are actually sent. > > Anyhow, it seems the only solution at the time being is set up a > custom test runner, which is what I ended up doing. However, it > occurred to me that a more elegant approach would be to use signals > that run during setup and teardown, which applications could hook into > if they needed to do any global setup or teardown actions. To me, this > seems like an excellent solution to the problem (it's actually what I > implemented in my custom test runner I ended up using). > > I wonder if this is something that would be considered for addition to > the core? I at least wanted to throw it out there for discussions. I don't remember this being raised in the past, but it seems like a reasonable proposal to me -- in particular, because of this: > * 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. This is the argument that resonates with me. The patch you provide is a good proof-of-concept, but it needs a little bit more work before it is trunk-ready. In particular: 1) For consistency, the existing setup/teardown code should use this signal. This then doubles as a test for the new signals, since the email tests won't pass unless the test setup signal is working. 2) The signal (and the resulting changes to setup/teardown procedures) need to be documented. Don't get too hung up on getting the language perfect -- a good first draft in the right location will suffice. There are two sets of documentation changes -- adding the test signals to the signal index, and documentation in the test docs about how/when to use the signal in practice. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.