Re: Proposal: Add signals test_setup and test_teardown to Django test suite runner

[Jim D.]

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

[Carl Meyer]

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

[Jim D.]

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

[Jim D.]

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

[Russell Keith-Magee]

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.

Proposal: Add signals test_setup and test_teardown to Django test suite runner

[Jim D.]

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've included a patch I wrote to implement this in the core test
module, at the bottom of this message. It should be mostly self
explanatory. Note that the call to send the test_setup figure has to
occur after get_apps() has executed in build_suite(), since
applications aren't loaded until then and any signal connections would
not yet have had a chance to be set up.

Just a few arguments to throw out in favor of this idea:

* 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 5 lines of additional code.

Anyhow, hopefully this is something you guys would be interested in
considering. Apologies if this topic has been discussed or proposed
before, but in searching I could not find anything related to it.

Proposed patch:


Index: test/simple.py
===
--- test/simple.py  (revision 13861)
+++ 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'
@@ -246,7 +247,11 @@
 else:
 for app in get_apps():
 suite.addTest(build_suite(app))
-
+
+# This signal can't come any earlier, because applications
are actually loaded
+# in get_apps()
+test_setup.send(sender=self)
+
 if extra_tests:
 for test in extra_tests:
 suite.addTest(test)
@@ -284,6 +289,7 @@
 connection.creation.destroy_test_db(old_name,
self.verbosity)

 def teardown_test_environment(self, **kwargs):
+test_teardown.send(sender=self)
 teardown_test_environment()

 def suite_result(self, suite, result, **kwargs):
Index: test/signals.py
===
--- test/signals.py (revision 13861)
+++ test/signals.py (working copy)
@@ -1,3 +1,6 @@
 from django.dispatch import Signal

 template_rendered = Signal(providing_args=["template", "context"])
+test_setup = Signal()
+test_teardown = Signal()
+

-- 
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.