Provide the ability for model definitions that are only availably during testing

Workaround:

import sys

if sys.argv[1] == 'test':
    # This is a bit hacky, see
    # http://code.djangoproject.com/ticket/7835
    from django.db import models

    class Poll(models.Model):
        # .. ..

A discussion came about on the user-list about this: http://groups.google.com/group/django-users/browse_thread/thread/559aa0a2d074a7b5

I wrote a patch based on Russell's feedback. The proposed API is as follows:

class MyMetaTest(TestCase):
    installed_apps = ['fakeapp','otherapp']
    extra_apps = ('yetanotherapp',)

    def test_stuff(self):
      ... 

• installed_apps and extra_apps can either be tuple or a list. They can coexist or be used individually.

• installed_apps overrides the settings INSTALLED_APPS.

• extra_apps adds the given apps either to INSTALLED_APPS or to installed_apps if it also exists.

Now, responding specifically to Russell's remarks:

"Obviously, the test applications need to be:

1. Added to INSTALLED APPS and the app cache on startup
2. Installed in the app cache before the syncdb caused by the pre-test database flush takes effect. You shouldn't need to manually invoke syncdb.
3. Removed from INSTALLED_APPS and the app cache on teardown"

My answers below:

1. Done
2. Done, but I think syncdb still needs to be invoked to create the extra tables, otherwise flush will raise an exception saying it cannot find those tables.
3. INSTALLED_APPS is properly restored, but I could not find a way to unload the apps from the cache. Should an extra method be written in the AppStore? class (unload_app() and/or unregister_models())?

Path with suggested new API (installed_apps and extra_apps)

Replying to julien:

2. Done, but I think syncdb still needs to be invoked to create the extra tables, otherwise flush will raise an exception saying it cannot find those tables.

This reveals a slightly larger bug, which I've logged as #9717. The right approach here is to fix #9717, not work around the issue.

3. INSTALLED_APPS is properly restored, but I could not find a way to unload the apps from the cache. Should an extra method be written in the AppStore? class (unload_app() and/or unregister_models())?

I would be highly surprised if you could deliver this patch without adding some cache cleanup methods to the AppStore?.

Also - I know this is early days, but just as a friendly reminder before I turn into the Patch Hulk: Patch need tests! Patch need docs! Aaargh! Hulk angry!!... (oh dear... too late! :-)

But how do you test a testing patch? One suggestion - migrate some of the existing Django tests to use the new framework. For example, contrib.admin has tests currently stored in the system tests that should probably be application tests. If you migrate the admin tests, this will demonstrate that your patch works and has sufficient capabilities for a real-world testing situation.

One issue that the documentation should address - some sort of convention to avoid application name clashes. manage.py validate will prevent model and table clashes, but if I name one of my test applications 'sites' or 'auth', all hell could break lose. You might want to suggest some form of convention around test application naming - for example, naming all test applications 'test_foo', rather than just 'foo'. Alterntaively, you could enforce this programatically by registering test applications with a test_ prefix (this could raise other complications though - it might be easier to stick with the convention).

Replying to russellm:

This reveals a slightly larger bug, which I've logged as #9717. The right approach here is to fix #9717, not work around the issue.

Ok, I'll look at that one, see if I can help.

I would be highly surprised if you could deliver this patch without adding some cache cleanup methods to the AppStore?.

Will look at that too.

Also - I know this is early days, but just as a friendly reminder before I turn into the Patch Hulk: Patch need tests! Patch need docs! Aaargh! Hulk angry!!... (oh dear... too late! :-)

Aahhh!! The Patch Hulk is back!!! :) In fact, I didn't want to write too much doc/tests before settling on the API. What's your view on this? Is the suggested API ok, or should it be different?

As you suggest, I'll try to migrate some admin tests. For the doc, I agree with you that some clear and well-explained conventions would be preferable. I'll introduce that in the next patch.

Deferring documentation is ok; writing good docs takes a lot of time, and it takes a lot of effort to rework if a big design change is made. I think the API is stable enough at this point to warrant making a start on documentation.

However, deferring tests isn't ok. Tests are how you prove to me that the code works, that you've though about all the nasty ways that things can go wrong, and that you are handling those exceptions.

Point taken about the tests ;) I had tested in-house with some of my apps, but it's true that without tests it's hard to prove to others that it works. I also wasn't sure how to write tests for this, but migrating admin tests seem like a good one.

Another question. Shouldn't ticket #9717 be merged with this one? After all, the issue emerged from this ticket and no one's really complained about that separately before (correct me if wrong). The same thing applies for adding unloading methods to AppStore, which at this stage wouldn't justify a ticket on its own.

My only concern is that all these issues (flush, AppStore and test apps) need to be fixed all at once. And while I'm happy to try to fix them (and for anyone else who'd be interested to contribute), managing 2 sets of tickets/patches would be more painful to handle.

What do you think? Just let me know, and I'll comply to whatever you decide ;)

No - #9717 shouldn't be merged, that's why I opened another ticket. Consider - #9717 can be fixed without requiring a fix for this ticket; likewise, this ticket could be closed without fixing #9717. Hence, they are separate tickets. However, I would say that fixing #9717 is a reasonable pre-requisite for merging this ticket, as any fix for this ticket will need to work around the problem identified by #9717.

There is also the branch management issue; a patch for #9717 will need to be applied to the v1.0.X branch because it is a bug fix; this ticket describes a feature addition, so it will only be applied to trunk.

Sure, not many people have complained about this problem, but that doesn't mean nobody has had the problem. It is entirely possible that people have been having the problem but not understanding the cause.

Regarding two patches being difficult to handle: If you provide a standalone patch for #9717, I will get it into the trunk with haste, as it is a clearly identified bug with existing code.

patch+test+doc

So I have finally migrated the tests for contrib.comments which seems a more doable intermediary step before migrating huge tests like the admin's. I also wrote a piece of documentation to present the new API and a suggested convention for avoiding app name clashes.

The only thing that was requested and which I haven't done is unloading the test apps from the AppCache. But, is it really necessary? It will be highly recommended to developers to follow a certain convention to avoid name clashes. If they do so, then unloading the models/apps from the cache won't be crucial to do. If they don't do so, then there might be a whole lot of unpredicted conflicts at several other levels anyway. Ideas?

Oh, another note. In the code patch, I still need to run syncdb to create potential new tables for the test apps' models. I don't think this can really be avoided. And I don't think this is a problem either.

Replying to julien:

Oh, another note. In the code patch, I still need to run syncdb to create potential new tables for the test apps' models. I don't think this can really be avoided. And I don't think this is a problem either.

The problem isn't (just) one of code neatness - it's the execution time for the tests. On my machine, the full Django's system test suite takes about 5 minutes to execute for SQLite; almost 10 minutes to run for Postgres. I haven't run the Oracle tests myself, but I've been lead to believe that it goes from "go make yourself a cup of coffee" to "go make yourself a 9-course degustation banquet". I'm going to be very picky about introducing anything that has the potential to make this situation worse.

Syncdb isn't a no-op when there is nothing to do. Given that there is a already a syncdb being called as part of the flush, my initial reaction is that you shouldn't need another one. This may mean that there are some other modifications that need to be made; I'm not opposed to making such changes, if they're required.

If an additional call to syncdb is completely unavoidable, then so be it; however, I'm not yet convinced that it is unavoidable. Feel free to convince me :-)

Replying to julien:

The only thing that was requested and which I haven't done is unloading the test apps from the AppCache. But, is it really necessary?

Again, feel free to convince me otherwise, but my initial reaction is "yes". Tests shouldn't have side effects after their execution; stale entries sticking around the app cache have the potential to introduce this sort of side effects.

Replying to russellm:

If an additional call to syncdb is completely unavoidable, then so be it; however, I'm not yet convinced that it is unavoidable. Feel free to convince me :-)

The proposed API allows one to add *any* app to INSTALLED_APPS, that is, some that may have already been synced (e.g. the common ones like contenttypes or auth) and some that probably haven't been synced yet (typically the app's internal test apps). Therefore I think that a synchronisation is necessary because we can't predict which apps have been synced or not yet.

You say that "there is a already a syncdb being called as part of the flush". This is not exactly true because my patch simply won't work without an explicit call to syncdb: flush does not create tables for the dynamically added apps which haven't been synced yet.

But even so, I don't think it would have such a latency impact. Assuming that this new API ever gets checked in, I presume only (or mostly) the contrib apps would bother using it so they become more self-contained (I'm talking in the context of the Django test suite). And, with the boost improvement scheduled for 1.1 (all tests run in one transaction), these considerations might well be negligible anyway.

Finally (and maybe more importantly) I am not sure how to go without syncdb :) Any hint?

Replying to russellm:

Again, feel free to convince me otherwise, but my initial reaction is "yes". Tests shouldn't have side effects after their execution; stale entries sticking around the app cache have the potential to introduce this sort of side effects.

I don't know if I can convince you for this one either, but I have one question: if the tests introduce such side effects, wouldn't that mean that the AppCache is buggy in the first place?

The test framework already relies on many conventions. As long as one sticks to those (and assuming the AppCache is not buggy), then I tend to think that everything would go fine. Obviously, if one is not careful enough and does not follow the conventions, then things could crash miserably.

Also, considering the proposed API, how would we know which app to unload, since we may include some apps that have already been loaded by the test suite (e.g. the most common ones like contenttypes or auth).

All these considerations depend on whether on not the suggested API is adequate. You haven't commented much on that yet. Could you share your thoughts on the quality of the API and how it could be improved?

The following is not a proposed changed since it is a hack, but here is what I have done to have models during testing:

In my app I have a folder "tests". It contains "testingmodels.py". In my test case (defined in tests/init.py) I have redefined _pre_setup() like this:

import sys
import new
from django.db.models import loading
from django.core.management import call_command
from django.test import TestCase

from . import testingmodels

class MyTestCase(TestCase)

    def _pre_setup(self):
        # register tests models
        # we'll fake an app named 'my_test_app'
        module = new.module('my_test_app')
        module.__file__ = __file__ # somewhere django looks at __file__. Feed it.
        module.models = testingmodels
        testingmodels.__name__ = 'my_test_app.models'
        sys.modules['my_test_app'] = module

        # register fake app in django and create DB models
        from django.conf import settings
        settings.INSTALLED_APPS += ('my_test_app',)
        loading.load_app('my_test_app')
        call_command('syncdb', verbosity=0, interactive=False)

        return super(MyTestCase, self)._pre_setup()

Note: due to django constraints the app must contains an empty "models.py".

I'm using this with success under django 1.0.2. I can't say I would recommend it as a general solution, but maybe it can help or inspire someone?

Thanks for this. I thought I'd also remind here the hack (originally posted at http://groups.google.com/group/django-users/browse_thread/thread/559aa0a2d074a7b5) which I've successfully used in many applications, and which is reasonably succinct in terms of lines of code. This is in fact what served as a model for the patch I've posted in this ticket. Maybe that will be useful to someone until this ticket eventually gets fixed.

Sample file structure:

    myapp/
        tests/
            fakeapp/
                __init__.py
                models.py
            __init__.py
            models.py
            views.py
            urls.py

Here is the testing code (located in myapp/tests/__init__.py):

    import sys

    from django.test import TestCase
    from django.conf import settings
    from django.core.management import call_command
    from django.db.models.loading import load_app
    
    from fakeapp.models import FakeItem
    
    class TestMyApp(TestCase):
    
        def setUp(self):
            self.old_INSTALLED_APPS = settings.INSTALLED_APPS
            settings.INSTALLED_APPS = (
                'django.contrib.auth',
                'django.contrib.contenttypes',
                'myapp',
                'myapp.tests.fakeapp',
            )
            load_app('myapp.tests.fakeapp')
            call_command('syncdb', verbosity=0, interactive=False) #Create tables for fakeapp

        def tearDown(self):
            settings.INSTALLED_APPS = self.old_INSTALLED_APPS

        def test_blah(self):
            item = FakeItem.objects.create(name="blah")
            #Do some testing here...