Skip to content

Handle setUpClass also for unittest.TestCase #197

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

Closed

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jan 20, 2015

This is a follow-up to 8a9aacb.

@pelme
Copy link
Member

pelme commented Jan 21, 2015

Hmm, what is the purpose of this? What use cases are there for plain unittest testcases that expect the database to be there? Such tests is likely already broken since they have no proper rollback/flush in an ordinary with Django's test runner.

If mixing non Django tests (which is unittest.TestCase subclasses) and having pytest-django enabled, this change may lead to unexpected things.

@blueyed
Copy link
Contributor Author

blueyed commented Jan 21, 2015

I've came across such a testcase (https://github.com/yourlabs/django-autocomplete-light/blob/master/autocomplete_light/tests/views.py#L28), and thought it might have been just an oversight that regular unittests are not handled by this in pytest-django.

I agree that the test could/should be fixed probably, e.g. by using a Django TestCase.

However, for the process of migrating a test suite to pytest/pytest-django it seems to be good to behave like Django's testrunner would (even when that's broken).

If mixing non Django tests (which is unittest.TestCase subclasses) and having pytest-django enabled, this change may lead to unexpected things.

Not sure about this.
Without this change the database is there/available already, but not wrapped like with Django's TestCases.

@pelme
Copy link
Member

pelme commented Jan 24, 2015

If you have a test suite with some unittest.TestCase subclasses (that does not require a Django database) and some tests which subclass from django.test.TestCase. Without this change, you can run py.test some_plain_unittest and have them run entirely without creating/connecting to the database.

Also, If you have a legacy test suite which interacts with the Django DB without inheriting from django.test.TestCase, the tests will explicitly fail and it will be clear that those tests are broken. Is there any other case where this change would be useful, except for tests which should be django.test.TestCase in the first place?

Regarding the error message about the marker, we could probably improve the failure when a database access happens from a unittest.TestCase subclass to instruct the user to subclass from django.test.TestCase instead.

@blueyed
Copy link
Contributor Author

blueyed commented Jan 24, 2015

Regarding the error message about the marker, we could probably improve the failure when a database access happens from a unittest.TestCase subclass to instruct the user to subclass from django.test.TestCase instead.

👍

@blueyed
Copy link
Contributor Author

blueyed commented Jan 24, 2015

So, this PR is void, and I'll close it once we have an issue for the above.

@blueyed blueyed force-pushed the handle-setUpClass-with-unittest branch from 8492f3c to cd7c898 Compare January 24, 2015 22:46
blueyed added a commit to blueyed/pytest-django that referenced this pull request Jan 24, 2015
@blueyed
Copy link
Contributor Author

blueyed commented Jan 24, 2015

@pelme
I've amended the commit to add a test for the current behavior instead.

Regarding the error message about the marker, we could probably improve the failure when a database access happens from a unittest.TestCase subclass to instruct the user to subclass from django.test.TestCase instead.

Should this then also happen for access through setUpClass, or should this behavior be left unchanged?

@blueyed
Copy link
Contributor Author

blueyed commented May 8, 2015

I've pushed the test case, and created a new issue about improving the failure case: #234

Closing this issue therefore.

@blueyed blueyed closed this May 8, 2015
@blueyed blueyed deleted the handle-setUpClass-with-unittest branch May 8, 2015 18:05
mfa pushed a commit to aexeagmbh/pytest-django that referenced this pull request May 17, 2017
beyondgeeks pushed a commit to beyondgeeks/django-pytest that referenced this pull request Sep 6, 2022
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.

2 participants