Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrations in django-taggit don't work when SOUTH_TESTS_MIGRATE = True (triggered by python_files=*.py) #158

Closed
jurecuhalev opened this issue Aug 13, 2014 · 10 comments

Comments

@jurecuhalev
Copy link

Fix for issue #128 introduced ability for SOUTH_TESTS_MIGRATE = True, to correctly run south migrations. In this specific case I want to run migrations since it provides data migration for data that I want to use in tests.

I'm trying to integrate it into one my projects that is using django-taggit and it crashes with strange south error:

(pytest-example)➜  blog git:(master) ./manage.py test
=========================================================================================== test session starts ============================================================================================
platform darwin -- Python 2.7.8 -- py-1.4.23 -- pytest-2.6.1
plugins: django
collected 1 items 

web/tests.py E

================================================================================================== ERRORS ==================================================================================================
_______________________________________________________________________________ ERROR at setup of test_ambassador_permission _______________________________________________________________________________

request = <SubRequest '_django_db_marker' for <Function 'test_ambassador_permission'>>

    @pytest.fixture(autouse=True)
    def _django_db_marker(request):
        """Implement the django_db marker, internal to pytest-django

        This will dynamically request the ``db`` or ``transactional_db``
        fixtures as required by the django_db marker.
        """
        marker = request.keywords.get('django_db', None)
        if marker:
            validate_django_db(marker)
            if marker.transaction:
                request.getfuncargvalue('transactional_db')
            else:
>               request.getfuncargvalue('db')

../../../.virtualenvs/pytest-example/src/pytest-django/pytest_django/plugin.py:179: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1337: in getfuncargvalue
    return self._get_active_fixturedef(argname).cached_result[0]
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1351: in _get_active_fixturedef
    result = self._getfuncargvalue(fixturedef)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1403: in _getfuncargvalue
    val = fixturedef.execute(request=subrequest)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1820: in execute
    fixturedef = request._get_active_fixturedef(argname)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1351: in _get_active_fixturedef
    result = self._getfuncargvalue(fixturedef)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1403: in _getfuncargvalue
    val = fixturedef.execute(request=subrequest)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1853: in execute
    self.yieldctx)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/_pytest/python.py:1779: in call_fixture_func
    res = fixturefunc(**kwargs)
../../../.virtualenvs/pytest-example/src/pytest-django/pytest_django/fixtures.py:49: in _django_db_setup
    db_cfg = setup_databases(verbosity=0, interactive=False)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/test/runner.py:279: in setup_databases
    verbosity, autoclobber=not interactive)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/hacks/django_1_0.py:103: in wrapper
    return f(*args, **kwargs)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/db/backends/creation.py:339: in create_test_db
    load_initial_data=False)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/core/management/__init__.py:159: in call_command
    return klass.execute(*args, **defaults)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/core/management/base.py:285: in execute
    output = self.handle(*args, **options)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/core/management/base.py:415: in handle
    return self.handle_noargs(**options)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/management/commands/syncdb.py:103: in handle_noargs
    management.call_command('migrate', **options)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/core/management/__init__.py:159: in call_command
    return klass.execute(*args, **defaults)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/django/core/management/base.py:285: in execute
    output = self.handle(*args, **options)
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/management/commands/migrate.py:111: in handle
    ignore_ghosts = ignore_ghosts,
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/migration/__init__.py:179: in migrate_app
    Migrations.invalidate_all_modules()
../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/migration/base.py:245: in invalidate_all_modules
    migration.invalidate_module()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <Migration: web:0001_initial>

    def invalidate_module(self):
        """
            Removes the cached version of this migration's module import, so we
            have to re-import it. Used when south.db.db changes.
            """
>       reload(self.migration())
E       ImportError: reload(): module web.migrations.0001_initial not in sys.modules

../../../.virtualenvs/pytest-example/lib/python2.7/site-packages/south/migration/base.py:389: ImportError
========================================================================================= 1 error in 0.30 seconds ==========================================================================================

I'm not sure if it's a problem in pytest_django or django-taggit, but my hunch is that you'll probably understand better what's going on with south mocking.

to make it easier to test, I've also created a GitHub repo: https://github.com/gandalfar/pytest-example
that is currently using f10e48d

@blueyed
Copy link
Contributor

blueyed commented Aug 18, 2014

Thanks for your report!

I have looked into it, but can only provide more information for the moment.

When adding some print statements, it appears that South calls invalidate_all_modules multiple times (once per app):

migrate_app taggit
invalidate_all_modules
  taggit:0001_initial
  taggit:0002_unique_tagnames
  web:0001_initial
  web:0002_auto__add_field_post_user
  web:0003_data_migration
migrate_app web
invalidate_all_modules
  taggit:0001_initial
=> ImportError with `reload`

(I have changed the order of INSTALLED_APPS for testing).

The odd thing is:

ipdb> e
ImportError('reload(): module taggit.south_migrations.0001_initial not in sys.modules',)
ipdb> sys.modules['taggit.south_migrations.0001_initial']
<module 'taggit.south_migrations.0001_initial' from '/home/daniel/.virtualenvs/tmp-66489d0828cf808/lib/python2.7/site-packages/taggit/south_migrations/0001_initial.py'>
ipdb> 'taggit.south_migrations.0001_initial' in sys.modules
True

I have noticed that _pytest/assertion/rewrite.py(50)find_module() gets called with the first call to reload, which might trigger something weird here.

I've only skimmed the documentation for reload/imp.reload, and it's apparently not that trivial (ref https://docs.python.org/2/library/functions.html#reload).

FWIW, I have tested it with pytest-master also, and used Python 2.7.8 in general (there were compatibility problems with python-3.4).

@blueyed blueyed added the bug label Aug 18, 2014
@pelme
Copy link
Member

pelme commented Aug 18, 2014

Out of curiosity: It seems that you run tests from ./manage.py test - how is that configured?

@blueyed
Copy link
Contributor

blueyed commented Aug 18, 2014

@blueyed blueyed changed the title Migrations in django-taggit don't work when SOUTH_TEST_MIGRATE = True Migrations in django-taggit don't work when SOUTH_TESTS_MIGRATE = True Aug 18, 2014
@blueyed
Copy link
Contributor

blueyed commented Aug 18, 2014

FWIW, I am also using django-taggit in my own project, and it does not trigger this issue.

I have just tried manage.py migrationcheck, which failed on the second app (reversion), after taggit, because the test database did not exist anymore (it was created after an interactive question).
(This appears to be another issue with South - running manage.py migrationcheck reversion by itself/alone works).

@blueyed
Copy link
Contributor

blueyed commented Aug 18, 2014

It is caused by python_files=*.py in your pytest.ini.

Commenting that line and moving web/tests.py to web/test_web.py works.

Although --collect-only only finds the single test:

py.test --collect-only blog web
… test session starts …
platform linux2 -- Python 2.7.8 -- py-1.4.23 -- pytest-2.5.2
plugins: django, ipdb
collected 1 items
<Module 'web/test_web.py'>
  <Function 'test_ambassador_permission'>

(same with pytest-2.6.1, just also tested it with 2.5)

@pelme
Copy link
Member

pelme commented Aug 25, 2014

So the problem is that the same file gets imported twice from different locations?

Is there anything we can do to prevent that? The original issue seems to have been resolved by changing pytyhon_files.

@blueyed
Copy link
Contributor

blueyed commented Aug 27, 2014

@pelme
I don't think that it's about imports, but have not checked/verified it.

I think it's some bad interaction between pytest magic and South magic probably, nothing pytest-django could do about probably.

@jurecuhalev
Copy link
Author

I think it might be made easier for newcomers through slightly different documentation.

If python_files=*.py, breaks south migrations, then this FAQ: http://pytest-django.readthedocs.org/en/latest/faq.html#my-tests-are-not-being-found-why-not
can maybe be rewritten to recommend renaming tests.py to tests_app.py instead?

blueyed added a commit to blueyed/pytest-django that referenced this issue Sep 30, 2014
This is meant to reproduce/test
pytest-dev#158, but does not
cause a failure.
blueyed added a commit to blueyed/pytest-django that referenced this issue Oct 2, 2014
This is meant to reproduce/test
pytest-dev#158, but does not
cause a failure.
blueyed added a commit to blueyed/pytest-django that referenced this issue Mar 2, 2015
This is meant to reproduce/test
pytest-dev#158, but does not
cause a failure.
@blueyed
Copy link
Contributor

blueyed commented Mar 2, 2015

@gandalfar
Can you provide feedback on if the workaround actually worked for you, and/or if you had any other insights?
(I've came across my PR/branch again and just rebased it on pytest-django master)

@blueyed blueyed changed the title Migrations in django-taggit don't work when SOUTH_TESTS_MIGRATE = True Migrations in django-taggit don't work when SOUTH_TESTS_MIGRATE = True (triggered by python_files=*.py) Mar 2, 2015
@blueyed
Copy link
Contributor

blueyed commented Mar 3, 2015

I've found a way to reproduce it in a test. The problem is triggered by also using django-taggit: blueyed@5ce816a

The bug is with pytest though, where the PEP 302 importer module breaks reload.

I've submitted a PR for pytest at: https://bitbucket.org/pytest-dev/pytest/pull-request/259/fix-reload-for-files-matched-by.

I am closing the issue for pytest-django.

@blueyed blueyed closed this as completed Mar 3, 2015
mfa pushed a commit to aexeagmbh/pytest-django that referenced this issue May 17, 2017
This is meant to reproduce/test
pytest-dev#158, but does not
cause a failure.
beyondgeeks added a commit to beyondgeeks/django-pytest that referenced this issue Sep 6, 2022
This is meant to reproduce/test
pytest-dev/pytest-django#158, but does not
cause a failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants