Pytest-django: pytest 5.4 skips Django's teardown (causing e.g. "connection already closed" errors)

[Fak3]

pytest-django==3.5.1
django versions tested: 2.2.11 and 3.0.4

When one test fails, all tests which use database launched after this test will fail with traceback:

test/apiv2/put_inputevent__test.py:34: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/model_mommy/mommy.py:54: in make
    return mommy.make(_save_kwargs=_save_kwargs, **attrs)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/model_mommy/mommy.py:228: in make
    return self._make(commit=True, commit_related=True, _save_kwargs=_save_kwargs, **attrs)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/model_mommy/mommy.py:265: in _make
    instance = self.instance(self.model_attrs, _commit=commit, _save_kwargs=_save_kwargs)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/model_mommy/mommy.py:289: in instance
    instance.save(**_save_kwargs)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/base.py:741: in save
    force_update=force_update, update_fields=update_fields)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/base.py:779: in save_base
    force_update, using, update_fields,
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/base.py:851: in _save_table
    forced_update)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/base.py:900: in _do_update
    return filtered._update(values) > 0
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/query.py:760: in _update
    return query.get_compiler(self.db).execute_sql(CURSOR)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/sql/compiler.py:1469: in execute_sql
    cursor = super().execute_sql(result_type)
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/models/sql/compiler.py:1138: in execute_sql
    cursor = self.connection.cursor()
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/backends/base/base.py:256: in cursor
    return self._cursor()
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/backends/base/base.py:235: in _cursor
    return self._prepare_cursor(self.create_cursor(name))
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/utils.py:89: in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/backends/base/base.py:235: in _cursor
    return self._prepare_cursor(self.create_cursor(name))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <django.db.backends.postgresql.base.DatabaseWrapper object at 0x7fe8150ed208>, name = None

    def create_cursor(self, name=None):
        if name:
            # In autocommit mode, the cursor will be used outside of a
            # transaction, hence use a holdable cursor.
            cursor = self.connection.cursor(name, scrollable=False, withhold=self.connection.autocommit)
        else:
>           cursor = self.connection.cursor()
E           django.db.utils.InterfaceError: connection already closed

/home/u1/.virtualenvs/npserver/lib/python3.7/site-packages/django/db/backends/postgresql/base.py:223: InterfaceError

Downgrading to pytest==5.3.5 fixes the issue.

Not yet sure if this is pytest issue or pytest-django.

[bluetech]

Can you confirm this happens also on pytest 5.4.1?

If yes, then a minimal reproduction would be great.

[Fak3]

Yes, 5.4.1 too. Will try to create minimal case.

[Fak3]

Created minimal django project: https://github.com/Fak3/testproject-824-git

[bluetech]

Thanks for the reproduction!

It looked to me like a problem in pytest's unittest support, so I looked at changes made to that in pytest in 5.4.x. Reverting https://github.com/pytest-dev/pytest/pull/5996 "unittest: do not use TestCase.debug() with --pdb" (and other commits in that PR) fixes the problem.

cc @blueyed @nicoddemus who worked on that PR and probably have a better idea than me.

[blueyed]

@Fak3
Please try https://github.com/pytest-dev/pytest-django/pull/825.
The problem is that Django's (rather custom) teardown gets not called (because this is throwing an exception to get out of unittest's __call__, so that --pdb can be used before any teardown - although you could also just add a pdb.set_tracce() and be done).

(It's the next round of the long-standing issue from 2016, and I still think it should have been reverted first at least instead (ref https://github.com/pytest-dev/pytest/pull/6014), until it gets done properly, e.g. via something like https://github.com/blueyed/pytest/pull/107))

pytest-django could intercept Django's _post_teardown (https://github.com/django/django/blob/1a09708dcb2f60ad5ca0a75b8de9619356f74ff6/django/test/testcases.py#L274), and do it after the call, but that would still require to first patch pytest to not use the current approach.

[Fak3]

Please try #825.

Yes, that PR fixes this issue

[BenjamenMeyer]

Is https://github.com/pytest-dev/pytest/issues/6947 related?

[blueyed]

Is pytest-dev/pytest#6947 related?

Yes, caused by it.

Fixed in pytest-django 3.9.0 now.

[PyB1l]

I think that the issue persist in pytest-django 3.9.0 #835

[hugos94]

I think that the issue persist in pytest-django 3.9.0 #835

Same problem here!

[nicoddemus]

@PyB1l or @hugos94, any chance of testing this with pytest's latest master? If the problem still persists, would appreciate a minimal project to reproduce the problem (the one provided in https://github.com/pytest-dev/pytest-django/issues/824#issuecomment-598857918 is now returning 404).

[PyB1l]

For my tests i managed to isolate the issue in Django==3.0.2. For Django===3.0.5 and all the lastet version of pytest, pytest-django, i have no issues. @hugos94 could you check your versions and let us know?

[hugos94]

For my tests i managed to isolate the issue in Django==3.0.2. For Django===3.0.5 and all the lastet version of pytest, pytest-django, i have no issues. @hugos94 could you check your versions and let us know?

I am using the following versions:

• Django==2.2.7
• pytest==5.4.1
• pytest-django==3.9.0

Apparently the problem only happens after making requests in views of the application. In tests that i use graphql, the error doesn't occur.

[nicoddemus]

pytest 5.4.2 is out with the fix I mentioned earlier. If people can try that version and see if this is still a regression, would appreciate it. 👍

[nathanprat]

Hello,

The problem seems to have reappeared:

• pip install -U pytest-django!=4.3.0 then pytest = 626 passed, 6 skipped, 70 warnings
• pip install -U pytest-django==4.3.0 then pytest = 8 failed, 618 passed, 6 skipped, 70 warnings

Only the 2 tests below have an issue, which is weird considering the high number of tests still working fine:

@pytest.mark.parametrize('email', (
        '[email protected]',
        '[email protected]',
        '[email protected]',
        '[email protected]',
        '[email protected]',
        't!#$%&es!#$%&[email protected]',
        # Those are NOT valid, but we accept them to avoid an overly
        # complicated regex. In any case, the true validation SHOULD be to send
        # a confirmation email.
        '[email protected]',
    ))
    @pytest.mark.django_db
    def test_email_valid(self, db_setup_company: Company, email: str) -> None:
        company = db_setup_company

        user = User.objects.create(email=email, company=company)

        assert list(user.building_ids) == []

    @pytest.mark.parametrize('email', (
        'aaaa@[email protected]',
        '@@@test@@@@tld-minus.com',
    ))
    @pytest.mark.django_db
    def test_email_invalid(self, db_setup_company: Company, email: str) -> None:
        company = db_setup_company

        with pytest.raises(IntegrityError) as exc:
            User.objects.create(email=email, company=company)

        assert 'violates check constraint "email_valid_chk"' in str(exc)

freeze.txt

[browniebroke]

I noticed this problem as well. Do you prefer to re-open this bug or create a new issue? FWIW, here is the list of my other dependencies.

django==3.0.14
pytest-cov==2.12.0
pytest-env==0.6.2
pytest-forked==1.3.0
pytest-freezegun==0.4.2
pytest-instafail==0.4.2
pytest-mock==3.6.1
pytest-socket==0.4.0
pytest-subtests==0.4.0
pytest-vcr==1.0.2
pytest-xdist==2.2.1
pytest==6.2.4

My Django version is different from @nathanprat (3.0 vs 3.2) so it doesn't seem relevant. I see 3 common points with the above freeze.txt:

pytest==6.2.4
pytest-django==4.3.0
pytest-mock==3.6.1

UPDATE:

after further investigation, I've created a minimal project with out issue, you can see the failure in the action executions.

I've tracked down the problem to a custom fixture in our project. Doesn't sounds like a regression of this bug after all.

[BenjamenMeyer]

@browniebroke if you would, could you explain why that particular fixture is causing the problem? Might help others with finding similar issues.

[browniebroke]

I don't know why (yet) unfortunately, I've just managed to isolate this as the root cause, but I didn't persist further with the upgrade due to lack of time.

In case it helps, this fixture is supposed to persist model data across test cases within a class, a bit like setTestData does for Django's TestCase class. It's working for simple use cases, but at times, I've seen some side effects across tests, which are not present with setTestData.

[nathanprat]

Thanks for the investigation.

The bug was indeed caused by a class-level db fixture.

I don't have time to investigate deeper right now, but the code is below if anyone is interested:

@pytest.fixture(name='class_db', scope='class')
def fixture_class_db(request: 'Any',
                     django_db_setup: 'Any',  # pylint: disable=unused-argument
                     django_db_blocker: 'Any') -> None:
    """
    To avoid the error with 'fixture_db_setup(db)':

    "ScopeMismatch: You tried to access the 'function' scoped fixture 'db'
    with a 'module' scoped request object, involved factories"

    We reimplement a 'db'-like fixture class-scoped.
    """
    # Copy-pasted from pytest_django.fixtures
    if "django_db_reset_sequences" in request.fixturenames:
        request.getfixturevalue("django_db_reset_sequences")
    if ("transactional_db" in request.fixturenames
            or "live_server" in request.fixturenames):
        request.getfixturevalue("transactional_db")
    else:
        _django_db_fixture_helper(
            request,
            django_db_blocker,
            transactional=False)

[bluetech]

@browniebroke If you change your code from django_db_blocker.unblock() to with django_db_blocker.unblock(): ... does it help any?

[browniebroke]

Thanks for the suggestion @bluetech. I tried your suggestion (I think) but no luck yet... Was it the solution you had in mind?

[bluetech]

Yep. Although it was actually also correct before, my eye just glanced over the addfinalizer calls. Since you're now using a context manager you should be able to remove the request.addfinalizer(django_db_blocker.restore). You can also convert the request.addfinalizer(functools.partial(TestCase._rollback_atomics, atomics)) to a regular call after the yield.

(I understand this doesn't actually fix your problem. What I think you are trying to achieve -- safe sharing of DB data beyond a single test -- is currently a missing feature in pytest-django)