Changeset r16053 breaks a few views tests

[Jannis Leidel]

Traceback:

(django-dev)~/Code/git/django/tests [git:master] $ python runtests.py --settings=test_sqlite views
Creating test database for alias 'default'...
Creating test database for alias 'other'...
..........EEE..........................................................................
======================================================================
ERROR: test_template_exceptions (regressiontests.views.tests.debug.DebugViewTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jezdez/Code/git/django/tests/regressiontests/views/tests/debug.py", line 48, in test_template_exceptions
    self.client.get(reverse('template_exception', args=(n,)))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 391, in reverse
    *args, **kwargs)))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 312, in reverse
    possibilities = self.reverse_dict.getlist(lookup_view)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 208, in _populate
    for name in pattern.reverse_dict:
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 197, in _populate
    for pattern in reversed(self.url_patterns):
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 279, in _get_url_patterns
    patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 274, in _get_urlconf_module
    self._urlconf_module = import_module(self.urlconf_name)
  File "/Users/jezdez/Code/git/django/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/urls.py", line 4, in <module>
    import customadmin
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/customadmin.py", line 8, in <module>
    import models, forms
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/models.py", line 860, in <module>
    admin.site.register(ChapterXtra1, ChapterXtra1Admin)
  File "/Users/jezdez/Code/git/django/django/contrib/admin/sites.py", line 98, in register
    validate(admin_class, model)
  File "/Users/jezdez/Code/git/django/django/contrib/admin/validation.py", line 63, in validate
    cls.__name__, idx, fpath
ImproperlyConfigured: 'ChapterXtra1Admin.list_filter[4]' refers to 'chap__book__promo' which does not refer to a Field.

======================================================================
ERROR: test_template_loader_postmortem (regressiontests.views.tests.debug.DebugViewTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jezdez/Code/git/django/tests/regressiontests/views/tests/debug.py", line 56, in test_template_loader_postmortem
    response = self.client.get(reverse('raises_template_does_not_exist'))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 391, in reverse
    *args, **kwargs)))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 312, in reverse
    possibilities = self.reverse_dict.getlist(lookup_view)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 208, in _populate
    for name in pattern.reverse_dict:
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 197, in _populate
    for pattern in reversed(self.url_patterns):
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 279, in _get_url_patterns
    patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 274, in _get_urlconf_module
    self._urlconf_module = import_module(self.urlconf_name)
  File "/Users/jezdez/Code/git/django/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/urls.py", line 4, in <module>
    import customadmin
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/customadmin.py", line 8, in <module>
    import models, forms
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/models.py", line 806, in <module>
    admin.site.register(Article, ArticleAdmin)
  File "/Users/jezdez/Code/git/django/django/contrib/admin/sites.py", line 86, in register
    raise AlreadyRegistered('The model %s is already registered' % model.__name__)
AlreadyRegistered: The model Article is already registered

======================================================================
ERROR: test_view_exceptions (regressiontests.views.tests.debug.DebugViewTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jezdez/Code/git/django/tests/regressiontests/views/tests/debug.py", line 43, in test_view_exceptions
    reverse('view_exception', args=(n,)))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 391, in reverse
    *args, **kwargs)))
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 312, in reverse
    possibilities = self.reverse_dict.getlist(lookup_view)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 208, in _populate
    for name in pattern.reverse_dict:
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 229, in _get_reverse_dict
    self._populate()
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 197, in _populate
    for pattern in reversed(self.url_patterns):
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 279, in _get_url_patterns
    patterns = getattr(self.urlconf_module, "urlpatterns", self.urlconf_module)
  File "/Users/jezdez/Code/git/django/django/core/urlresolvers.py", line 274, in _get_urlconf_module
    self._urlconf_module = import_module(self.urlconf_name)
  File "/Users/jezdez/Code/git/django/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/urls.py", line 4, in <module>
    import customadmin
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/customadmin.py", line 8, in <module>
    import models, forms
  File "/Users/jezdez/Code/git/django/tests/regressiontests/admin_views/models.py", line 806, in <module>
    admin.site.register(Article, ArticleAdmin)
  File "/Users/jezdez/Code/git/django/django/contrib/admin/sites.py", line 86, in register
    raise AlreadyRegistered('The model %s is already registered' % model.__name__)
AlreadyRegistered: The model Article is already registered

----------------------------------------------------------------------
Ran 87 tests in 2.260s

FAILED (errors=3)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
(django-dev)~/Code/git/django/tests [git:master] $ 

[Carl Meyer]

Interesting, these tests pass when the full suite is run, but fail when just the view tests are run. Suggests a problem with the tests, I'll take a look.

[Carl Meyer]

Ok, two separate problems here. Both are fixed by this github branch, one in each commit: ​https://github.com/carljm/django/compare/master...t15903

First problem is that I missed a case in r16053, and not-installed models were being left out of any related models' related-objects cache. Meaning that two related models would look different (no reverse relation introspectable through _meta) if not installed than if installed. First commit fixes this, with a test (and adds a couple tests to flesh out the r16053 tests).

Second problem is that the views tests, when run standalone, were causing various other test apps to be imported due to url-reversing. This caused the first problem to show up in admin_views (because its models were being imported although it wasn't installed) only when the views tests were run without the admin_views tests. The second commit in the above branch fixes that particular test-app coupling by using the "urls" parameter of a TestCase.

I'll commit these separately for clarity, but I'll wait for a review from jezdez first, since this touches the app-cache and may affect work on the app-loading branch.

[Carl Meyer]

In [16106]:

Fixed #15903 -- Allowed not-installed models to still be referenced in related fields. Missed case from r16053.

[Carl Meyer]

In [16107]:

Refs #15903 -- Added a per-TestCase urlconf to reduce coupling between test apps.

[Luke Plant]

Carl:

I think there is still an issue with this. I've experienced some very difficult to debug problems with a project of mine, since [16053]. The problem only occurs when running the devserver with DEBUG=True (which cause the validation routines to run, I think this is connected), and appears to be some ManyToMany fields missing from the model ._meta definition. All of the models involved are installed AFAICS, but span a number of apps. The behaviour changes if I change the order of items in INSTALLED_APPS. Since the latest change, [16107], things have improved, in that with the right order of INSTALLED_APPS I can actually get things to work, but it should work in any order.

(I haven't been able to produce a minimal test case yet - I think that will require a lot of work).

But based on your last change, I tried the following in the _fill_related_many_to_many_cache() method:

--- a/django/db/models/options.py       Wed Apr 27 15:47:16 2011 +0000                 
+++ b/django/db/models/options.py       Wed Apr 27 18:18:58 2011 +0100                 
@@ -420,7 +420,7 @@                                                                    
                     cache[obj] = parent                                               
                 else:                                                                 
                     cache[obj] = model                                                
-        for klass in get_models():                                                    
+        for klass in get_models(include_auto_created=True, only_installed=False):     
             for f in klass._meta.local_many_to_many:                                  
                 if f.rel and not isinstance(f.rel.to, str) and self == f.rel.to._meta:
                     cache[RelatedObject(f.rel.to, klass, f)] = None                   

and it did fix my problem. It also worked with get_models(only_installed=False).

However, I haven't had the time to really understand your changes here or what is going on to know if this is the correct fix (or indeed, if it is actually a problem with my project's code).

[Carl Meyer]

Hi Luke,

Replying to lukeplant:

I think there is still an issue with this. I've experienced some very difficult to debug problems with a project of mine, since [16053]. The problem only occurs when running the devserver with DEBUG=True (which cause the validation routines to run, I think this is connected), and appears to be some ManyToMany fields missing from the model ._meta definition. All of the models involved are installed AFAICS, but span a number of apps. The behaviour changes if I change the order of items in INSTALLED_APPS. Since the latest change, [16107], things have improved, in that with the right order of INSTALLED_APPS I can actually get things to work, but it should work in any order.

(I haven't been able to produce a minimal test case yet - I think that will require a lot of work).

Sorry about this, and thanks for the report. It is indeed just another call to get_models that should be using only_installed=False that I missed; your one-line fix is correct (without changing include_auto_created, which isn't related). I thought I'd grepped for all these calls and checked them, but clearly I failed - I've done so again and this time I think I've got them all for real!

I'm not sure why this is showing up in your code with all the relevant apps in INSTALLED_APPS, or why it would be dependent on order of INSTALLED_APPS. The app-cache fully populates itself at the beginning of get_models, so any models whose app is in INSTALLED_APPS should be present in the return value of get_models(only_installed=True). Filling the related cache should use only_installed=False regardless, so these relationships are still reported in _meta even for not-installed models, which makes it a moot issue (although it would still be interesting to know why those related models weren't being returned with only_installed=True if they are in INSTALLED_APPS, if you can narrow it down at all).

[Carl Meyer]

Fixed this additional case in r16108, fat-fingered the Refs in the commit message.

[Aymeric Augustin]

get_models(only_installed=False) is next on my list of things to refactor -- read: remove -- in the new app loading. I'd like to get rid of this complexity (#21677) and eventually deprecate the ability to use models that aren't in an installed application (#21680).

There's no conclusive answer on why Luke hit that last problem, but I believe it has to do with a complex import sequence, where get_models was called on a partially populated app cache. Our best chance to improve the situation is to move the population of the app cache earlier (#21676) so that all models get imported from populate_models() -- rather than having populate_models() triggered by the import of a models module.

[Aymeric Augustin <aymeric.augustin@…>]

In 9f13c3328199d2fa70235cdc63bb06b1efc5b117:

Removed the only_installed argument of Apps.get_models.

Refs #15903, #15866, #15850.