jazzband / django-nose

Django test runner using nose
http://pypi.python.org/pypi/django-nose
BSD 3-Clause "New" or "Revised" License
882 stars 239 forks source link

Models located in tests.py are not created #15

Open kmmbvnr opened 14 years ago

kmmbvnr commented 14 years ago

Models that exists only for testing purpose are not created when tests running by django_nose

Problem exists because setup_databases called before any test module have been imported.

This could be fixed by moving setup_databases call into prepareTest in Plugin

shanx commented 14 years ago

I've mitigated the problem in one of our projects by simple defining my own testrunner that adds the required behaviour:

from django_nose.runner import NoseTestSuiteRunner

class PatchedNoseTestSuiteRunner(NoseTestSuiteRunner):
    def setup_test_environment(self, **kwargs):
        super(PatchedNoseTestSuiteRunner, self).setup_test_environment()
        # Unfortunately this is required for Django to correctly import models in tests and generate the database
        # structure for these models
        suite = self.build_suite(test_labels=None, extra_tests=None)
rozza commented 14 years ago

Now that setup_databases is in a plugin is this still an issue?

kmmbvnr commented 14 years ago

Hm, looks strange, but issue still exists

I created fresh application with following tests.py content

class TestModel(models.Model):
     name = models.CharField(max_length=20)

class SimpleTest(TestCase):
    def test_create_succeed(self):
        TestModel.objects.create(name='aaa')

with django_nose.NoseTestSuiteRunner receive: DatabaseError: no such table: testapp_testmodel

with standard django test runner tests passed

i used latest django-nose version from git

rozza commented 13 years ago

I've created a work around:

https://github.com/rozza/django-nose/commit/2cbc87170eb99df162d87ef040c9ae429dfea182

MOVED TO:

https://github.com/sbook/django-nose/commit/2cbc87170eb99df162d87ef040c9ae429dfea182

kmmbvnr commented 13 years ago

Hm, ever work around not helps.

Checkout: git://gist.github.com/763156.git

For me "./manage/py test" not succed neither for jbalogh or rozza django_nose

But it works for with usual django test runner (just comment out the TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' line in settings.py)

rozza commented 13 years ago

kmmbvnr - for my hack to work you need to add a setting: TEST_INSTALLED_APPS Which should include the test models. Its not nice I'm afraid :(

peterbe commented 13 years ago

I stumbled on this too. I tried what @shanx did but it didn't work. Here's how I fixed it for my project:

# settings.py
#TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'
TEST_RUNNER = 'runner.PatchedNoseTestSuiteRunner'

# runner.py
import django_nose
class PatchedNoseTestSuiteRunner(django_nose.NoseTestSuiteRunner):
   def run_tests(self, test_labels, extra_tests=None):
       self.build_suite(test_labels, extra_tests)
       return super(PatchedNoseTestSuiteRunner, self).run_tests(
         test_labels=test_labels, extra_tests=extra_tests)

@jbalogh, what do you think? Should this be part of stock django_nose? Wanna pull request?

peterbe commented 13 years ago

Sorry. I spoke to soon. This subclass gets app discovery wrong. What doesn't work even with this is the ability to delete instances of models defined and created inside the tests. Suppose, again, that you have this test:

class TestModel(models.Model):
   name = models.CharField(max_length=20)

class SimpleTest(TestCase):
    def test_create_succeed(self):
        instance = TestModel.objects.create(name='aaa')
        instance.delete() # where shit hits the fan

The problem appears to be because related objects in the models _meta aren't set up properly with using django_nose. When django deletes it uses instance._meta.get_all_related_objects() to delete related objects too (or whatever it does with them) and this is breaking when using django_nose.

The error I get is this one:

DatabaseError: no such table: nose_c

and as far as I can work out this happens because, the django.db.base.Model's delete method tries, incorrectly, to extract all related models which it does by a cache proxy pattern. What it finds is a related model for the model created in (any) tests.py that is basically a duplicate model. Running Django's stock test runner this model is created:

django_nose_issue_15.tests.TestModel

and it has, as you can expect two fields: one AutoField (the primary key) and CharField (name) (see above for kmmbvnr sample code) but when running with django_nose one more model is created which looks like this:

django_nose_i15.django_nose_issue_15.tests.TestModel

It has just one field which is a OneToOneField. Where the hell did that come from!? Basically, doing the extra build_suite() in the patched runner seems to cause excessive model creation. However, without it the relevant database tables aren't created. I've found an awful hack to go around this problem and it looks like this:

 class TestModel(models.Model):
    name = models.CharField(max_length=20)

 # calling this upon being imported means its internal cache gets it right
 TestModel._meta.get_all_related_objects()

 class SimpleTest(TestCase):
     def test_create_succeed(self):
         instance = TestModel.objects.create(name='aaa')
         instance.delete() # where shit hits the fan

If you do the above hack the internal cache gets correctly set before the actual test runner starts. So, it seems that there is something non-trivial that django.test.simple.DjangoTestSuiteRunner does when it sets up that the django_nose one doesn't do.

jbalogh commented 13 years ago

The root cause is that django-nose runs database setup in a nose plugin's begin method, and that happens before tests get collected. Your tests.py files haven't been imported so the models don't exist and they won't get db tables.

def begin(self):                                                                      
    """Called before any tests are collected or run. Use this to
    perform any setup needed before testing begins.
    """
rozza commented 13 years ago

My hack / fix has been moved to: https://github.com/sbook/django-nose/commit/2cbc87170eb99df162d87ef040c9ae429dfea182 and requires TEST_INSTALLED_APPS to be added to your settings.

dcramer commented 13 years ago

@rozza can you see if your use case works in master? I think we covered all of ours and it should also automatically pick up test-only applications assuming they're structured like a normal Django app

reinout commented 13 years ago

@dcramer: I tried out master with my models-in-tests.py project and they're not picked up.

I can't find anything with grepping and reading the readme that suggests how to set it up. Separate app? But that means I have to put it in the INSTALLED_APPS list (which I don't want). And I can't find something like @rozza's TEST_INSTALLED_APPS.

mallison commented 12 years ago

I'd just like to reask @reinout 's question. Is there a way to get test-only models created without putting the test app in INSTALLED_APPS?

erikrose commented 12 years ago

I'm having a swing at this.

schacki commented 12 years ago

I am sorry guys, but it still does not work for me, still get the error message, that the underlying model table does not exist. Am I supposed to use any specific nose TestCase class or can I use the standard Django TestCase class?

Brant commented 12 years ago

@schacki The updates worked fine for me, test models are loading in as expected from tests.py.

Make sure the updated package is the one being read on your pythonpath - if you are using virtualenv, for example, you might still have the old django-nose in your site-packages or something and it may be taking precedence over a localized copy of the new version. Also make sure any old .pyc files are cleaned up.

P.S. thanks dev's - this was an enormous help!

zvictor commented 12 years ago

not working for me.

test_models.py:

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

class MyModel(models.Model):
    one_field = models.CharField(max_length=100)

class MyModelTest(TestCase):
    def test_create_succeed(self):
        MyModel.objects.create(one_field='aaa')

output:

DatabaseError: relation "tests_mymodel" does not exist
LINE 1: INSERT INTO "tests_mymodel" ("one_field") VALUES ('aaa') RET...
zvictor commented 12 years ago

I am doing further tests, and I observed some things.

myapp/
├── __init__.py
├── models.py
├── tests.py (tests and test models here)
└── views.py

if I run ./manage.py test myapp, everything works fine, but if I run ./manage.py test we have the issue.

myapp/
├── __init__.py
├── models.py
└── tests/
       ├── __init__.py
       └── test_models.py (tests and test models here)

Both commands doesn't works. Only if I put models inside tests/init.py ./manage.py test myapp works.


I believe it is somehow related to #77

fabiosantoscode commented 11 years ago

@zvictor I'm experiencing that behavior. Models in tests.py will work if I do manage.py test app, but not on "full" tests (manage.py test).

mattissf commented 11 years ago

Confirming this issue.

Works Declaring the test model in __init__.py in the same package as the tests AND running manage.py test <app>

Does not work Declaring the test model in __init__.py in the same package as the tests AND running manage.py test Declaring the test model in the same file as the tests

Will only get "DatabaseError: no such table: <table_name>"

Relevant environment

Python 2.7.3 Django==1.4.3 django-nose==1.1 nose==1.2.1

ccorbacho commented 11 years ago

Digging through nose, the problem is that if you don't specify the modules to run the tests on, nose uses nose.loader.TestLoader.loadTestsFromDir() to do the discovery and creation of nose.suite.ContextSuite(). This function is a generator, so it does not import all the tests up front. This means that database setup happens before all the tests have been found.

If you specify the modules up front that you want to test, then nose creates all the nose.suite.CreateSuite() objects up front via nose.loader.TestLoader.lostTestsFromModule(). Database setup then happens after this. The original fix for this issue only fixed creating the test models in this case.

I think the fix for the test discovery case would be to add a new plugin to django-nose that provides a non-lazy version of loadTestsFromDir().

fabiosantoscode commented 11 years ago

Is a plugin really the best option, seeing that this is just plain incorrect behaviour?

I think, test models are mostly important for reusable apps. Should reusable apps' tests suites break if a project-wide setting (the activation of this plugin) is triggering a bug? On 2 Mar 2013 00:48, "Carlos Corbacho" notifications@github.com wrote:

Digging through nose, the problem is that if you don't specify the modules to run the tests on, nose uses nose.loader.TestLoader.loadTestsFromDir() to do the discovery and creation of nose.suite.ContextSuite(). This function is a generator, so it does not import all the tests up front. This means that database setup happens before all the tests have been found.

If you specify the modules up front that you want to test, then nose creates all the nose.suite.CreateSuite() objects up front via nose.loader.TestLoader.lostTestsFromModule(). Database setup then happens after this. The original fix for this issue only fixed creating the test models in this case.

I think the fix for the test discovery case would be to add a new plugin to django-nose that provides a non-lazy version of loadTestsFromDir().

— Reply to this email directly or view it on GitHubhttps://github.com/jbalogh/django-nose/issues/15#issuecomment-14319596 .

ccorbacho commented 11 years ago

What makes nose's behaviour incorrect though? There's nothing wrong with being lazy in the test discovery and evaluation in the general case, it's only with Django that we care about doing the test discovery upfront because we only want to do the database setup and teardown once, not per test.

If I were nose, I don't see why I would want to change the behaviour just for Django.

fabiosantoscode commented 11 years ago

Nose is being inconsistent right now. Run tests one way, you get one effect. Run tests the other, you get a different effect. Both manners of running tests should yield the same results. On 2 Mar 2013 11:01, "Carlos Corbacho" notifications@github.com wrote:

What makes nose's behaviour incorrect though? There's nothing wrong with being lazy in the test discovery and evaluation in the general case, it's only with Django that we care about doing the test discovery upfront because we only want to do the database setup and teardown once, not per test.

If I were nose, I don't see why I would want to change the behaviour just for Django.

— Reply to this email directly or view it on GitHubhttps://github.com/jbalogh/django-nose/issues/15#issuecomment-14326490 .

fabiosantoscode commented 11 years ago

I get your point now.

I'm not trying to say that this is an upstream issue from nose. I fear I've used the name "nose" instead of "django-nose". I apologize for the confusion.

That said, a plugin seems like a fine idea.

fabiosantoscode commented 11 years ago

This is the docstring of loadTestsFromDir:

    """Load tests from the directory at path. This is a generator
    -- each suite of tests from a module or other file is yielded
    and is expected to be executed before the next file is
    examined.
    """

I am afraid that importing modules upfront is going to cause some unexpected behavior for anything upstream that depends on this. I don't think there is an option here, and I believe the risk is neglectible. I'm going to try to fix this and pull request anyway, to see what everyone else thinks.

This problem has made me clutter my fabfiles for long enough.

fabiosantoscode commented 11 years ago

I've made a pull request https://github.com/jbalogh/django-nose/pull/111 to fix this.

If anyone could try it out and give feedback as to whether it fixes their tests, it would help integration.

GeorgeErickson commented 11 years ago

Adding an app_label to the test only model fixes the problem for me.

ivirabyan commented 11 years ago

The issue is closed, but I still experience this problem. It only works for me when I specify the app I want to test.

fabiosantoscode commented 11 years ago

@GeorgeErickson very interesting. Maybe there's a better fix to be made for this issue. @ivirabyan you can use my pull request #111 but it's kind of old now, I suspect there's been some progress upstream of my fork.

anentropic commented 11 years ago

How is this supposed to work? it doesn't seem to work for me.

in my test_models.py I have at the top:

class DummyModel(models.Model):
    class Meta:
        app_label = 'tests'

I do ./manage.py test myapp.tests.test_models but I just get errors

DatabaseError: no such table: tests_dummymodel

I have a recent pypi install of django-nose

anentropic commented 11 years ago

I could only get this working by this hack: http://stackoverflow.com/a/2672444/202168

which is TestCase subclass which adds the tests module to INSTALLED_APPS... then you put the DummyModel into tests/models.py

AdrianLC commented 11 years ago

Hello,

Is this feature working? The README says so but I'm struggling to test a simple abstract model. Freshly installed:

My file structure:

app_name
├── __init__.py
├── models.py (and views.py, etc.)
└── tests
        ├── __init__.py
        └── test_models.py
# In models.py:

class OwnedModel(models.Model):
    owner_user = models.ForeignKey(User, null=True, 
                                   related_name="%(app_label)s_%(class)s_ownerships")
    class Meta:
        abstract = True

# In tests/test_models.py:

class _OwnedModelTestModel(OwnedModel):
    class Meta:
        app_label = 'app_name'

class OwnedModelTestCase(TestCase):
    def setUp(self):
        self.instance = _OwnedModelTestModel()
        self.instance.save()

The traceback:

======================================================================
ERROR: test_can_be_owned_by_an_user (app_name.tests.test_models.OwnedModelTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/adrian/cluster_site/remote_credentials/tests/test_models.py", line 20, in setUp
    self.instance.save()
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/models/base.py", line 546, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/models/base.py", line 650, in save_base
    result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/models/manager.py", line 215, in _insert
    return insert_query(self.model, objs, fields, **kwargs)
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/models/query.py", line 1675, in insert_query
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 937, in execute_sql
    cursor.execute(sql, params)
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/backends/postgresql_psycopg2/base.py", line 58, in execute
    six.reraise(utils.DatabaseError, utils.DatabaseError(*tuple(e.args)), sys.exc_info()[2])
  File "/home/adrian/.virtualenvs/clusters/lib/python2.7/site-packages/django/db/backends/postgresql_psycopg2/base.py", line 54, in execute
    return self.cursor.execute(query, args)
DatabaseError: no existe la relación «app_name__ownedmodeltestmodel»
 # ^--> this is "relationship doesn't exist" in spanish
LINE 1: INSERT INTO "app_name__ownedmodeltestmodel" ("own...
cansin commented 10 years ago

I am getting the exact same error with @anentropic . Should we just try to hack it for now?

GeorgeErickson commented 10 years ago

@anentropic and @cansin, the app_label attribute needed to be of an existing app in the project, so using app_label = 'test' won't work because test isn't an app.

utkjad commented 9 years ago

Guys, do we have a fix for this now? Reiterating the question - Can we create Model Class for testing without putting it in INSTALLED_APPS, and execute/test it successfully?

jwhitlock commented 9 years ago

Reopening since many people struggle with this.

For my own reusable apps, I add a sample app and project to the repo, and run django-nose against the sample project. See the django-nose repo, and https://github.com/jwhitlock/drf-cached-instances.

Because there are work arounds, this feels like a feature request, not a bug.

avanheuvelen commented 8 years ago

If this is considered a feature request, than perhaps the documentation should be updated.

The docs specify that this behavior works while clearly there are some caveats: http://django-nose.readthedocs.org/en/latest/usage.html#test-only-models

oppianmatt commented 7 years ago

Is there any traction on this as still getting the error. The docs say it's possible. Normal django tests do it. It's clearly not a feature but a bug as it goes against the documentation and expected behaviour.