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

Add Django main to tox envlist. #953

Merged
merged 1 commit into from Nov 15, 2021
Merged

Conversation

hramezani
Copy link
Member

No description provided.

@hramezani
Copy link
Member Author

hramezani commented Nov 5, 2021

@bluetech again we have a problem in testing with django main. Here is the error log:

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

    @pytest.fixture(autouse=True)
    def _django_db_marker(request) -> None:
        """Implement the django_db marker, internal to pytest-django.
    
        This will dynamically request the ``db``, ``transactional_db`` or
        ``django_db_reset_sequences`` fixtures as required by the django_db marker.
        """
        marker = request.node.get_closest_marker("django_db")
        if marker:
            transaction, reset_sequences, databases = validate_django_db(marker)
    
            # TODO: Use pytest Store (item.store) once that's stable.
            request.node._pytest_django_databases = databases
    
            if reset_sequences:
                request.getfixturevalue("django_db_reset_sequences")
            elif transaction:
                request.getfixturevalue("transactional_db")
            else:
>               request.getfixturevalue("db")

pytest_django/plugin.py:472: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pytest_django/fixtures.py:254: in db
    _django_db_fixture_helper(request, django_db_blocker, transactional=False)
pytest_django/fixtures.py:177: in _django_db_fixture_helper
    PytestDjangoTestCase.setUpClass()
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/test/testcases.py:1173: in setUpClass
    cls.cls_atomics = cls._enter_atomics()
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/test/testcases.py:1150: in _enter_atomics
    atomics[db_name].__enter__()
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/db/transaction.py:189: in __enter__
    if not connection.get_autocommit():
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/db/backends/base/base.py:400: in get_autocommit
    self.ensure_connection()
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/utils/asyncio.py:25: in inner
    return func(*args, **kwargs)
.tox/py39-djmain-sqlite-coverage/lib/python3.9/site-packages/django/db/backends/base/base.py:230: in ensure_connection
    self.connect()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.test.testcases._DatabaseFailure object at 0x7fafe41d8670>

    def __call__(self):
>       raise AssertionError(self.message)
E       AssertionError: Database connections to 'replica' are not allowed in this test. Add 'replica' to pytest_django.fixtures._django_db_fixture_helper.<locals>.PytestDjangoTestCase.databases to ensure proper test isolation and silence this failure.

I've checked the _django_db_fixture_helper function and it seems it sets PytestDjangoTestCase.databases correctly.
We can see this behaviour after this commit in Django django/django@faba5b7
Do you have any idea?

@bluetech
Copy link
Member

bluetech commented Nov 7, 2021

Thanks for the PR @hramezani! It might take me a while to look at the failure, but I'll definitely check it out. From the error it is related to the experimental multi-db support, which also causes some (flaky?) sqlite failures in other PRs. Any investigation you can make (e.g. what changed in that django commit) would be great.

@hramezani hramezani force-pushed the django_main branch 4 times, most recently from 5597c92 to c971205 Compare November 15, 2021 20:13
@hramezani
Copy link
Member Author

@bluetech I think we had a problem with Django main because they removed SimpleTestCase.tearDownClass in django/django@faba5b7.

I've added it to PytestDjangoTestCase and made it available for Django >- 4.0

Also, it seems we have a problem with Github action.

PytestDjangoTestCase.setUpClass()
if VERSION >= (4, 0):
request.addfinalizer(PytestDjangoTestCase.django_tear_down_class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the following work, instead of copying django_tear_down_class over?

Suggested change
request.addfinalizer(PytestDjangoTestCase.django_tear_down_class)
request.addfinalizer(PytestDjangoTestCase.doClassCleanups)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, perfect!

@hramezani
Copy link
Member Author

Not sure py35-dj22-postgres-pytest54-coverage fails related to this patch or not.

@bluetech
Copy link
Member

Not sure py35-dj22-postgres-pytest54-coverage fails related to this patch or not.

No, it looks like more-itertools/more-itertools#578, which will hopefully resolve itself.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hramezani! I will be aiming for a release this weekend.

@bluetech bluetech merged commit c5465ad into pytest-dev:master Nov 15, 2021
@hramezani hramezani deleted the django_main branch November 15, 2021 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants