zsoldosp / django-currentuser

Conveniently store reference to request user on thread/db level.
BSD 3-Clause "New" or "Revised" License
139 stars 40 forks source link

tearDown problems in unit tests? #14

Closed vincentwhales closed 5 years ago

vincentwhales commented 6 years ago

I have the following model:

class Business(models.Model):
    ... 
    created_on = models.DateTimeField(auto_now_add=True, editable=False)
    created_by = CurrentUserField(null=True, blank=True, editable=False,       # Attention 4
                                  on_delete=models.SET_NULL)

And I am testing this in the following test case:

class Base(TestCase):
    def setUp(self):
        self.padmin = User.objects.create_user('padmin')
        for u in (self.padmin,):
            u.set_password('password')
            u.save()

        self.business = Business.objects.create(name='name', address='address',
                                                phone_number='123-456-7890')
        self.business2 = Business.objects.create(name='name2', address='address',
                                                 phone_number='123-456-7891')

    def STEP_create_instance(self):
        self.client.login(username='padmin', password='password')
        url = '/'
        self.client.post(url, {})    # Attention: 1

class SafesiteProductionFlowIntegrationTest(Base):

    def test_one(self):
        self.STEP_create_instance()    # Attention: 2

    def test_two(self):
        self.STEP_create_instance()    # Attention: 3 

When I run this test, I am getting the following error:

System check identified no issues (0 silenced).
..E
======================================================================
ERROR: test_before_split2 (instances.tests.test_viewflow.SafesiteProductionFlowIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 83, in _execute
    return self.cursor.execute(sql)
psycopg2.IntegrityError: insert or update on table "businesses_business" violates foreign key constraint "businesses_business_created_by_id_25175862_fk_auth_user_id"
DETAIL:  Key (created_by_id)=(1) is not present in table "auth_user".

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/test/testcases.py", line 209, in __call__
    self._post_teardown()
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/test/testcases.py", line 893, in _post_teardown
    self._fixture_teardown()
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/test/testcases.py", line 1041, in _fixture_teardown
    connections[db_name].check_constraints()
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/postgresql/base.py", line 235, in check_constraints
    self.cursor().execute('SET CONSTRAINTS ALL IMMEDIATE')
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 68, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 77, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 85, in _execute
    return self.cursor.execute(sql, params)
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 83, in _execute
    return self.cursor.execute(sql)
django.db.utils.IntegrityError: insert or update on table "businesses_business" violates foreign key constraint "businesses_business_created_by_id_25175862_fk_auth_user_id"
DETAIL:  Key (created_by_id)=(1) is not present in table "auth_user".

----------------------------------------------------------------------
Ran 2 tests in 0.438s

FAILED (errors=1)

Commenting out the following removes the error:

It seems like CurrentUserField is causing some issues during tearDown.

Any ideas?

belugame commented 6 years ago

Hey @vincentwhales, thank you for your input. Can you add a super(Base, self).setUp() to your own setUp method and see if that changes anything. It looks to me like the db-cleanup after a test is not working properly. If it doesn't change the outcome it would help us, if you give us your view code for / and your test database config.

You are using django.test.TestCase?

vincentwhales commented 6 years ago

Hi @belugame

So I've added super().setUp (python 3) to the beginning of my setUp and it didn't work.

For your information, I've changed the url path to something that results in 404 and still produce the error.

    def STEP_create_instance(self):
        self.client.login(username='padmin', password='password')
        url = '/thisdoesntevenexist'
        self.client.post(url, {})

This still results in the same error, so I doubt my code in / matters.

Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.....WARNING:django.request:Not Found: /thisdoesnteventhavetoexist
.WARNING:django.request:Not Found: /thisdoesnteventhavetoexist
.E
======================================================================
ERROR: test_before_split2 (instances.tests.test_viewflow.SafesiteProductionFlowIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/vng/.virtualenvs/prometheus/lib/python3.6/site-packages/django/db/backends/utils.py", line 83, in _execute
    return self.cursor.execute(sql)
psycopg2.IntegrityError: insert or update on table "businesses_business" violates foreign key constraint "businesses_business_created_by_id_25175862_fk_auth_user_id"
DETAIL:  Key (created_by_id)=(1) is not present in table "auth_user".
kairichard commented 6 years ago

hi @vincentwhales the problem seems to be that the user you create gets deleted at some point before the teardown thus causing the error not present in table "auth_user". (could be django's transactional TestCase) Since you create the User in setup I would for once try to move the creation of the user inside test_before_split2

Hope that helps.

vincentwhales commented 6 years ago

@kairichard I gave your recommendation a shot - unfortunately, it didn't work.

However, I poked around even more and discovered the following:

    def STEP_create_instance(self):
        #self.client.login(username='padmin', password='password')
        url = '/'
        self.client.post(url, {})

If i comment out self.client.login, both my tests actually works. If I leave it uncommented, my test fails with the same error. I definitely think CurrentUserField is having issues with self.client.login

Do you have any clues?

For reference:

class Base(TestCase):
    def setUp(self):
        self.padmin = User.objects.create(username='padmin')
        for u in (self.padmin,):
            u.set_password('password')
            u.save()

        self.business = Business.objects.create(name='name', address='address',
                                                phone_number='123-456-7890')
        self.business2 = Business.objects.create(name='name2', address='address',
                                                 phone_number='123-456-7891')

    def STEP_create_instance(self):
        #self.client.login(username='padmin', password='password')
        url = '/'
        self.client.post(url, {})

class SafesiteProductionFlowIntegrationTest(Base):
    def test_before_split(self):
        self.STEP_create_instance()

    def test_before_split2(self):
        self.STEP_create_instance()

and definition of Business

class Business(models.Model):
    ... 
    created_on = models.DateTimeField(auto_now_add=True, editable=False)
    created_by = CurrentUserField(null=True, blank=True, editable=False,       # Attention 4
                                  on_delete=models.SET_NULL)
vincentwhales commented 6 years ago

so it turns out all I need is to have

def tearDown(self):
        _set_current_user(None)

and this problem is fixed.

Is this expected behavior? If so, please include it in the doc so others won't face the same issue as me.

kairichard commented 6 years ago

I know we have that in the testcases for this package - it still seems strange to me since all this does is free up the variable from the locals of a module. Which database engine do you use for testing?

kairichard commented 6 years ago

@vincentwhales I am unable to reproduce this locally - which python version, requirements and database engine are you using during tests?

vincentwhales commented 6 years ago

@kairichard

Python 3.6.3

Database:

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.postgresql',
        'NAME': 'prometheus',
        'HOST': '127.0.0.1',
        'PORT': '5432',
    }
}

My requirements file:

-e git+https://github.com/qdqmedia/django-actions.git@76750bf815aa7c3de3e65506d4b78576f92b1287#egg=django_actions
Collectfast==0.6.2
Django==2.0.4
Faker==0.8.12
Jinja2==2.10
MarkupSafe==1.0
PyNaCl==1.2.1
PyYAML==3.12
Pygments==2.2.0
WooCommerce==1.2.1
amqp==2.2.2
appnope==0.1.0
asn1crypto==0.24.0
bcrypt==3.1.4
beautifulsoup4==4.6.0
billiard==3.5.0.3
boto3==1.7.9
botocore==1.10.9
celery==4.1.0
certifi==2018.1.18
cffi==1.11.5
chardet==3.0.4
cryptography==2.2.2
decorator==4.2.1
diff-match-patch==20121119
dj-database-url==0.5.0
django-autocomplete-light==3.2.10
django-bootstrap4==0.0.6
django-celery-beat==1.1.1
django-celery-results==1.0.1
django-compat==1.0.15
django-contrib-comments==1.8.0
django-currentuser==0.2.2
django-extensions==2.0.6
django-filter==1.1.0
django-hijack-admin==2.1.7
django-hijack==2.1.7
django-import-export==1.0.0
django-material==1.2.2
django-reversion==2.0.13
django-storages==1.6.6
django-tables2==2.0.0a2
django-tinymce==2.7.0
docutils==0.14
et-xmlfile==1.0.1
fabric==2.1.3
flake8==3.5.0
funcy==1.10.3
git+git://github.com/vincentwhales/viewflow.git
git+git://github.com/vincentwhales/wp-api-python.git
gunicorn==19.7.1
idna==2.6
invoke==1.0.0
ipython-genutils==0.2.0
ipython==6.2.1
isort==4.3.4
jdcal==1.3
jedi==0.11.1
jmespath==0.9.3
kombu==4.1.0
lxml==4.2.0
mccabe==0.6.1
nltk==3.2.5
oauthlib==2.0.6
odfpy==1.3.6
openpyxl==2.5.1
ordereddict==1.1
paramiko==2.4.1
parso==0.1.1
pexpect==4.4.0
pickleshare==0.7.4
pip-autoremove==0.9.1
prompt-toolkit==1.0.15
psycopg2==2.7.4
ptyprocess==0.5.2
pyasn1==0.4.3
pycodestyle==2.3.1
pycparser==2.18
pyflakes==1.6.0
python-dateutil==2.7.1
pytz==2018.4
redis==2.10.6
requests-oauthlib==0.8.0
requests==2.18.4
s3transfer==0.1.13
simplegeneric==0.8.1
six==1.11.0
slackclient==1.2.1
tablib==0.12.1
text-unidecode==1.2
traitlets==4.3.2
unicodecsv==0.14.1
urllib3==1.22
vine==1.1.4
wcwidth==0.1.7
websocket-client==0.47.0
whitenoise==3.3.1
xlrd==1.1.0
xlwt==1.3.0
kairichard commented 6 years ago

@vincentwhales great, thanks. This should enable us to at least reproduce the problem.

benjhastings commented 5 years ago

I ran into this same problem and resolved it by adding the following to conftest.py (I'm using pytest and pytest-django). This way I don't have to worry about calling _set_current_user(None) in a tearDown() on every test - the plugin does it for me.

from django_currentuser.middleware import _set_current_user

class UnsetCurrentUserPlugin:
    def pytest_runtest_teardown(self, item):
        _set_current_user(None)
        return

def pytest_configure(config):
    config.pluginmanager.register(UnsetCurrentUserPlugin(), "unset_user_plugin")  # noqa
aidanlister commented 4 years ago

I wasn't sure how to use @benjhastings code (eg how does django know to call pytest_configure?) but I used this:

We have a "BaseTestClass" that we use everywhere, so I added this as a mixin to that base class:

class ThreadLocalTestHelperMixin:
    """ This fixes a bug (or shortcoming) in django-currentuser

    https://github.com/PaesslerAG/django-currentuser/issues/14
    """
    def tearDown(self):
        _set_current_user(None)
jakubste commented 4 years ago

I run across this problem and spend over 2 days to figure what is breaking our tests (which was quite not obvious since we've got two databases setup and we thought that it was causing problems with integrity).

I think that absolute minimum is to mention this issue in the readme and maybe provide at least TestCase subclass/mixin that clears currect_user.

I don't even know why this issue is closed since no real solution was provided, only a workaround.

benjhastings commented 4 years ago

@aidanlister pytest_configure is a hook that gets called after command line options have been parsed. See https://docs.pytest.org/en/latest/reference.html#_pytest.hookspec.pytest_configure

agger-magenta commented 3 years ago

I encountered this problem today and had some trouble finding out what to do, esp. since each individual test passed, even individual test suites all passed, the failure only occurred when running all tests at once.

In the end I did an explicit _ set_current_user_() in all test classes, i.e. in their setUp function.

I agree with @jakubste that it might be a good idea to mention this in the readme, but I don't have time ATM to send a PR.

EverWinter23 commented 1 year ago

This particular issue made me question my skills. I wasn't even aware of this package being used in the project, I thought it was a DB issue or my fixtures were setup incorrectly! I ended up wasting so much time, check the DB constraints.

Thank you @vincentwhales for the solution.