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

[Russell Keith-Magee]

A current limitation of the unit test framework is that there is no capacity to define 'test models' - that is, models that are only required for the purposes of testing. A regular installation would not know anything about these models - only a test database would have access to them.

There are several existing applications that have a need for this capability: For example:

• contrib.admin: you can't test the way admin handles models without some models to handle.
• contrib.databrowse: you can't test the way the browser works without having models to browse
• Django Evolution: you can't evolve models without having some models to evolve.

The easiest way to work around this at present is to have a standalone test project which exercises the required functionality. However, these tests aren't integrated into the automated test suite, so they.

Another option is to do some app_cache munging during the test - this works, but is very messy.

Django should provide a way for model definitions to be defined as part of the test definition, synchronized as part of the test setup, populated and manipulated during test execution, and destroyed along with the test database.

[peritus <peritus@…>]

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):
        # .. ..

[Julien Phalip]

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())?

[Julien Phalip]

Path with suggested new API (installed_apps and extra_apps)

[Russell Keith-Magee]

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

[Julien Phalip]

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.

[Russell Keith-Magee]

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.

[Julien Phalip]

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 ;)

[Russell Keith-Magee]

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.

[Julien Phalip]

patch+test+doc

[Julien Phalip]

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?

[Julien Phalip]

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.

[Russell Keith-Magee]

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 :-)

[Russell Keith-Magee]

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.

[Julien Phalip]

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?

[Julien Phalip]

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?

[bmathieu]

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?

[Julien Phalip]

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

[Carl Meyer]

So, I'm a little surprised this hasn't been mentioned here already, which makes me wonder if I'm missing something obvious, but: in the process of checking out #14677 (and associated django-users post, ​https://groups.google.com/group/django-users/browse_thread/thread/3574e98ac6d3a774/f9bfde1741155272), it appears to me that we already have a pretty good working solution for test-only models in trunk (and I'm wondering why I never thought of it). Apparently you can simply define models directly in your tests.py. Syncdb never imports tests.py, so those models won't get synced to the normal db, but they will get synced to the test database, and can be used in tests. I haven't done this extensively myself (yet), but I just tested and it works. Problem solved?

The only difference I can see between that and the new feature being discussed here is whether the test-only models live in the same app as the one under test, or in a special test-only app. But I can't think of any reasons having them in the same app would be a problem. It even seems potentially a bit cleaner.

Of course, we still may want a fix for #14677, then; which parallels #4470, which we're hoping will be fixed when Arthur Koziel's GSOC branch is merged to fix #3591? I don't know whether that branch will otherwise impact this strategy for test-only models.

[Jari Pennanen]

I don't know what is the current suggested syntax for this, but either way it should be defined inside the app, and not like in the example of description. Probably it should be in the __init__.py of app, with something like:

APP_LABEL='myauth'

I mean it should not be variable.

E.g. if we allow the app_label be variable, what happens to all permission checks? Someone installs the auth app as myauth and suddenly all permission checks stops working.

[Jari Pennanen]

Uh wrong ticket guys, sorry, I was talking about #3591.

[Russell Keith-Magee]

@carljm; you're right that it hasn't been mentioned, but it hasn't been neglected, either.

To my mind, there are (a least) three features missing from the "put models in tests.py" technique:

• Specify only specific models for a specific test (e.g., only have the Article model for this particular test)
• Cleanup of contenttypes, permissions -- and most importantly -- the app cache itself.
• Defining multiple test apps. For example, to test the admin, you need multiple apps to demonstrate the app index works.

My original motivation here was to provide a test environment that was rich enough to support tests for model migrations. For that, you need all three features.

[Julien Phalip]

To second Russell's comment, my own personal motivation with this ticket were (and still are) to reproduce all the conditions necessary to test the dependency of an app with another fictional, self-contained, app (i.e. not just to test the other apps' models, but also its urls, its views, its templatetags, etc.).

For what's it's worth, I've actually successfully used the hack above in multiple production apps (e.g. ​http://github.com/glamkit/glamkit-blogtools/blob/master/blogtools/tests/__init__.py#L70). The main theoretical thing that's missing for me is a proper cleanup of the appcache to make sure that nothing goes ugly while running the rest of your project's test suite (although in practice I've never run into anything problematic so far).

[Adrián López Calvo]

With 1.6 and the new discovery-runner the models in tests.py are not being installed anymore, so this feature would be way more useful now.

Just leaving an idea here, what about a decorator just like override_settings but for installing models?

The usage would be like:

class SomeTestModel(models.Model):
    # whatever


class AnotherTestModel(models.Model):
    # whatever


class YetAnotherTestModel(models.Model):
    # whatever


@test_models(SomeTestModel)
class SomeModelsTestCase(TestCase):

    @test_models(AnotherTestModel)
    def test_them(self):
        with test_models(YetAnotherTestModel):
            # The 3 models available here

Of course this wouldn't actually install the app, just models for testing custom fields or abstract models.

I actually have some code done but it's far from being correctly tested and I don't think I am qualified enough to provide a good implementation.

Anyway, anticipating some issues I encountered:

• Main problem was the automatic rollback from TestCase. My database (postgres) rejects transactions with changes to a table followed by a table drop.
• When applied to a test method (or as a context manager), I couldn't find a way to detect if the test function comes from a TransactionTestCase or a TestCase.
• Finally managed something with restore_transaction_methods and disable_transaction_methods from django.test.testcases.

[JocelynD]

Replying to adrian_lc:

With 1.6 and the new discovery-runner the models in tests.py are not being installed anymore, so this feature would be way more useful now.

I don't get it, I did a minimal test case (attached) with django 1.6:

• a "buggy" project
• a "tested" app
• tested/models.py contains a single abstract model AbstractTestStat
• tested/tests.py contains a TestStat model inheriting from AbstractTestStat
• tested/tests.py contains a single test testing TestStat

Either

./manage.py test

or

./manage.py test tested

Works fine, populating the test db and succeeding test as expected.

[JocelynD]

However, I don't know why but when using south, the "Test Models in tests.py" trick does not work (table is not created), so you have to put in your settings.py the following :

SOUTH_TESTS_MIGRATE = False

to disable the south migrations for the test db.

(which will anyway save you some cpu time...)

[glassglaze@…]

Replying to JocelynD:

I don't get it, I did a minimal test case (attached) with django 1.6:

• a "buggy" project
• a "tested" app
• tested/models.py contains a single abstract model AbstractTestStat
• tested/tests.py contains a TestStat model inheriting from AbstractTestStat
• tested/tests.py contains a single test testing TestStat

Works fine, populating the test db and succeeding test as expected.

It seems like you don't interact with db (save models to test db). I didn't manage to run tests. Do you think I've missed something.

[Tim Graham]

I've marked #22459 "Creating model classes for test purposes breaks migrations" as a duplicate of this.

[mitchell]

I managed to work around this using migrations instead of syncdb under Django 1.7 with the following:

from django.conf import settings
from django.db import migrations


IS_TEST_DB = settings.DATABASES.get(
    'default', {}).get('NAME', '').startswith('test_')


class Migration(migrations.Migration):

    dependencies = [...]

    operations = [...]

    if IS_TEST_DB:
        operations.extend([
            ...
            ])

Alternatively you could use ​post_migrate hooks.

[Daniel Samuels]

This functionality is still broken as of Django 1.7.7. This is a very basic repro example:

from django.db import models
from django.test import TestCase


class TestModel(models.Model):
    label = models.CharField(
        max_length=100,
    )


class Test(TestCase):

    def setUp(self):
        TestModel.objects.create(
            label='X'
        )

Add this to any app as tests.py and run it, you will get an error like this:

django.db.utils.ProgrammingError: relation "media_testmodel" does not exist
LINE 1: ...a_testmodel"."id", "media_testmodel"."label" FROM "media_tes...

[Tim Graham]

A ​documentation patch has been proposed based on comment 38's approach of conditionally adding migration operations based on the database name. I don't think this approach is something we should officially recommend. I'd rather wait for a proper solution. One idea is a @test_model model class decorator that the migrations system respects.

[Ashley Waite]

This was a particularly annoying issue to me, which I resolved by creating a new TestRunner that did the work for me.

I think including something like this for easy use would effectively resolve this.

from importlib.util import find_spec
import unittest

from django.apps import apps
from django.conf import settings
from django.test.runner import DiscoverRunner


class TestLoader(unittest.TestLoader):
    """ Loader that reports all successful loads to a runner """
    def __init__(self, *args, runner, **kwargs):
        self.runner = runner
        super().__init__(*args, **kwargs)

    def loadTestsFromModule(self, module, pattern=None):
        suite = super().loadTestsFromModule(module, pattern)
        if suite.countTestCases():
            self.runner.register_test_module(module)
        return suite


class RunnerWithTestModels(DiscoverRunner):
    """ Test Runner that will add any test packages with a 'models' module to INSTALLED_APPS.
        Allows test only models to be defined within any package that contains tests.
        All test models should be set with app_label = 'tests'
    """
    def __init__(self, *args, **kwargs):
        self.test_packages = set()
        self.test_loader = TestLoader(runner=self)
        super().__init__(*args, **kwargs)

    def register_test_module(self, module):
        self.test_packages.add(module.__package__)

    def setup_databases(self, **kwargs):
        # Look for test models
        test_apps = set()
        for package in self.test_packages:
            if find_spec('.models', package):
                test_apps.add(package)
        # Add test apps with models to INSTALLED_APPS that aren't already there
        new_installed = settings.INSTALLED_APPS + tuple(ta for ta in test_apps if ta not in settings.INSTALLED_APPS)
        apps.set_installed_apps(new_installed)
        return super().setup_databases(**kwargs)

[Simon Charette]

While working on a workaround for this I came up with a non-invasive solution that might be acceptable to resolving the ticket.

The idea is similar to Ashley's solution but is more explicit as it require a function call in the app/tests/__init__.py module. It does however isolate each app into their own app, which prevent name collisions, and doesn't require the app_label = 'test' assignment on each test model.

The solutions boils down to this function

def setup_test_app(package, label=None):
    """
    Setup a Django test app for the provided package to allow test models
    tables to be created if the containing app has migrations.

    This function should be called from app.tests __init__ module and pass
    along __package__.
    """
    app_config = AppConfig.create(package)
    app_config.apps = apps
    if label is None:
        containing_app_config = apps.get_containing_app_config(package)
        label = f'{containing_app_config.label}_tests'
    if label in apps.app_configs:
        raise ValueError(f"There's already an app registered with the '{label}' label.')
    app_config.label = label
    apps.app_configs[app_config.label] = app_config
    app_config.import_models()
    apps.clear_cache()

Which when called from app/tests/__init__.py as setup_test_app(__package__) will create an app_tests appconfig entry and auto-discover the models automatically. Since the *.tests modules should only be loaded on test discovery the app and its models will only be available during tests. Keep in mind that if your test models reference models from an application with migrations you'll also need to manually create migrations for these tests models but once that's done you should be good to go.

It does feel less magic and convention based than Ashley's solution as it prevents conflicts between models and allows multiple test apps per app from any test package structure. Thoughts?

[Ahmad Abdallah]

Your solution is indeed less magic and much more isolated; reusing automatic model discovery from appConfig makes a lot more sense than attempting load them manually.

This will be a nice feature to have for 3.2.

The manual secondary migrations for models referenced outside of the test app seems like a fair cost to get models inside tests, and from my understanding, the test models are meant to be self-contained i.e models created purely for tests that will reference each other only so it shouldn't be much of an issue.

[Filipe Pina]

Thank you Simon for that clean life saving piece of code!
Been using for a few months and just today had to use a test child model of a real model so bumped into the lack of migrations.
Just defined it in the main models, ran makemigrations and moved that new migration into “tests/migrations”, worked perfectly.

Hope this makes it into a release “soon” :)

[HarryKane]

Replying to Simon Charette:

Keep in mind that if your test models reference models from an application with migrations you'll also need to manually create migrations for these tests models but once that's done you should be good to go.

Could you elaborate on how to manually create the migration for the test model?

[Josh Thomas]

Hard to believe this ticket hasn't been picked up yet. I've been using Simon's provided workaround for at least two years, maybe longer.

Any idea what would be needed for this to make it in to Django proper?

[Simon Charette]

@HarryKane, sorry I missed your comment

Could you elaborate on how to manually create the migration for the test model?

I had to manually create the migration files myself. Since the models are only auto-discovered during tests it's kind of painful to trick the migration system in creating the migration itself; you need to perform test auto-discovery before running makemigrations. Maybe it can be a limitation of the solution at first?

@Josh Thomas

Any idea what would be needed for this to make it in to Django proper?

Since no other solution emerged for that long I'd suggest creating a PR with the above solution available from django.test and documenting its usage. I've used it with success in many projects and I've been wanting to create third-party package that exposes it as well as a createtestmigrations command that basically performs test discovery and then delegates to makemigrations but I haven't been able to do so.

The challenge will likely be in writing tests for it. I guess we could models defined in a specialzed test app that are discovered by the Django test suite itself and that would be enough?