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

Order in which tests are executed #214

Open
blueyed opened this Issue Feb 27, 2015 · 20 comments

Comments

Projects
None yet
9 participants
@blueyed
Copy link
Contributor

blueyed commented Feb 27, 2015

pytest-django could / should probably use the same test ordering like Django: https://docs.djangoproject.com/en/1.7/topics/testing/overview/#order-in-which-tests-are-executed

This would mean to run all tests using the db fixture before tests using the transactional_db fixture, and then the remaining ones.

Django's tests should be run according to the Django documentation linked above.

This could be achieved using pytest's pytest_collection_modifyitems probably.

Some related pytest plugin: https://github.com/ftobia/pytest-ordering

@blueyed blueyed added the enhancement label Feb 27, 2015

@pelme

This comment has been minimized.

Copy link
Member

pelme commented Mar 29, 2015

I don't see very strong reasons to reorder the tests, but I'm not really against it either if it is easy to pull of.

I'm however strongly in favor of having tests that can run in any order. In my opinion, tests that doesn't clean up after themselves or tests that rely on state from other tests is broken and I think it is fine for pytest-django to be incompatible from Djangos test runner in those cases.

Here are some background discussions about ordering of tests in Django:
https://groups.google.com/forum/#!msg/django-developers/PfK0km66CNI/JbReB0pDkR8J
https://code.djangoproject.com/ticket/18271

blueyed added a commit to blueyed/pytest-django that referenced this issue Mar 29, 2015

Run tests in the same order as Django
This uses `pytest_collection_modifyitems` to order the tests.

Fixes pytest-dev#214.

@blueyed blueyed referenced this issue Mar 29, 2015

Open

Ordering of tests #223

0 of 2 tasks complete
@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Mar 29, 2015

See #223.

I'm however strongly in favor of having tests that can run in any order.

This could be done / tested using https://github.com/klrmn/pytest-random, which uses the same pytest hook and therefore order probably matters when used with the patch from #223.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Mar 29, 2015

Maybe we could add pytest-random (or the idea from there) to our Travis test suite?

@pelme

This comment has been minimized.

Copy link
Member

pelme commented Apr 1, 2015

The pytest-django test suite itself is probably not really random safe - there are some tests that kind of expects order (i.e. all test_*_again tests). I regard these as bugs, and these tests should definitely be replaced with proper testdir.runpytest() tests to make sure they are actually testing the right thing.

I am against re-ordering tests to conform with Django's order, the order should not make any difference with regard to compatibility anyways. I think re-ordering of tests is better left to pytest itself (possible future optimizations by being smarter about fixture caching etc), pytest-random and the pytest-xdist scheduler unless there is a strong reason to change the order.

I feel bad about this since you have put a lot of work into this and created a very nice patch, but I think we're better of by not merging this.

What are your thoughts about this?

@dan-passaro

This comment has been minimized.

Copy link

dan-passaro commented May 29, 2015

I'm personally +1 on this. I agree with you @pelme that, ideally, the order shouldn't matter. But here I am trying to chase down how we have a test that works fine on its own in but breaks when run after another specific test. This extra ramp-up time in getting pytest-django running doesn't look good for me or my decision to try and switch to pytest (for right now anyway, I'm sure once I get it up and running all will be forgiven).

If this pain on my end can be avoided by just taking up the same ordering as Django, I think that's a net win. It'll make it easier for existing Django projects to switch, even if they have mistakes in their test suites which are masked by the order the Django runner is executing their suite.

As un-ideal as it is, it is practical. Even having the option to explicitly allow re-ordering would be nice, that way I can show my team that when run the normal way everything appears good but... shuffle the order around and look at all the hidden bugs we have in our test suite!! Then we can plan time to address and fix that while not having that block our adoption of pytest and pytest-django.

@alvinchow86

This comment has been minimized.

Copy link

alvinchow86 commented May 29, 2015

At least, it's not too hard to do your own ordering with pytest_collection_modifyitems.

The application I had for this was that my test suite has some expensive DB fixtures that are set up for every test, and it was an enormous (probably 2-3x) overall performance gain to set up the fixtures first, then run each test (each one is already wrapped in a transaction), then run any tests requiring transactional database (e.g. @pytest.mark.transactional_db) at the end of the test run. Although to get this to work, the test reordering was the easiest thing..

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented May 29, 2015

@dan-passaro
Have you tried #223?

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented May 29, 2015

Looking at #223 it seems like it should also handle Django's test classes, and not only look at the pytest-specific markers.

@bparker98

This comment has been minimized.

Copy link

bparker98 commented Mar 14, 2016

We have found that after upgrading to Django 1.8 our tests that inherited from TransactionTestCase were causing our other tests to fail due to the database being rolled back to an empty state after TransactionTestCase finishes.

Adding serialized_rollback = True to our TransactionTestCase instances allowed those tests to run, but when another test that inherited from Django's TestCase ran after running a TransactionTestCase it would fail because the database would be empty again.

Adding serialized_rollback = True to all of our test cases slowed things down so much it was not an option.

So there are side-effects to not maintaining the same order.

Are there best practices we should be looking into to avoid this issue?

@penoux

This comment has been minimized.

Copy link

penoux commented Sep 2, 2016

+1 on previous comment from @bparker98. We are switching from Django TestRunner to Pytest Django, but we have not yet rewritten all our TestCase and TransactionTestCase to pytest. When Pytest executes our suite, the database is flushed after last TransactionTestCase (with serialized_rollback = True). The following TestCase then fails at each time. Django's strategy (running all TestCase, then all TransactionTestCase) may not be something to follow in the long term, but without it, Pytest brings a lot of regression (in our case though).

@rlucioni

This comment has been minimized.

Copy link

rlucioni commented Jan 30, 2017

I was struggling to switch an existing project over to pytest-django because of this issue. I took @blueyed's approach from #223, extended it to support Django's TransactionTestCase subclasses, turned it into a plugin, and put it on PyPI: https://pypi.python.org/pypi/pytest-django-ordering. If you want to use pytest-django but preserve the order in which Django runs tests, you can simply install the plugin alongside pytest-django.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Jan 30, 2017

@rlucioni
Thanks.
Do you think it would be easy to integrate it, based on an option?

@rlucioni

This comment has been minimized.

Copy link

rlucioni commented Jan 31, 2017

@blueyed sure, we'd just need to add a new Boolean option to pytest_addoption (e.g., --django-ordering) and check it before sorting collected tests. That said, if #223 wasn't merged, I'm not sure if this would be, either.

@pelme would the option described above make you more amenable to merging something like #223?

blueyed added a commit to blueyed/pytest-django that referenced this issue May 16, 2017

Run tests in the same order as Django
This uses `pytest_collection_modifyitems` to order the tests.

Fixes pytest-dev#214.

blueyed added a commit to blueyed/pytest-django that referenced this issue Dec 25, 2017

Run tests in the same order as Django
This uses `pytest_collection_modifyitems` to order the tests.

Fixes pytest-dev#214.
@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Dec 25, 2017

I've fixed/rebased #223 now, and it should be good as is then, but from the previous comment(s) from @rlucioni I think it could/should get improved further.

@rlucioni
How is the experience with your self-contained plugin?
Do you feel like adopting #223, and improving it so that your plugin would not be needed addtionally anymore?

@ostcar

This comment has been minimized.

Copy link

ostcar commented Jul 29, 2018

I would really like to see this in pytest-django. But to support pytest 3.6 you have to consider rlucioni/pytest-django-ordering#3

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Jul 29, 2018

@ostcar
Would be cool if you could help to get it in then.

I think the test from #223 could be used, and then the code extended from what pytest-django-ordering does.

I think we should default to ordering tests like Django probably then - not sure if a flag is needed (in the beginning).

I have not compared #223 to pytest-django-ordering, but at least TransactionTestCase seems to be missing (unittest based).

A good approach might be to create a new PR for this then, and we would close #223 then.

I guess @rlucioni would be happy if we would integrate it.

blueyed added a commit to blueyed/pytest-django that referenced this issue Jul 30, 2018

Run tests in the same order as Django
This uses `pytest_collection_modifyitems` to order the tests.

Fixes pytest-dev#214.
@rlucioni

This comment has been minimized.

Copy link

rlucioni commented Jul 30, 2018

@blueyed I believe pytest-django-ordering supports TransactionTestCase. The approach taken there is similar to the one in #223.

The experience of using the plugin is fine. Installing it is easy. Realizing you need it in the first place is less so. I have neither the time nor the inclination to adopt #223. If @ostcar would like to, I'm all for it.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Jul 30, 2018

@rlucioni
Thanks for your feedback!

Let's hope for @ostcar to pick it up - otherwise it appears to be quite a good bitesize issue for people wanting to contribute (given that the code exists already).

@blueyed blueyed added the bitesize label Jul 30, 2018

@ostcar

This comment has been minimized.

Copy link

ostcar commented Jul 31, 2018

I would like to make the pull request. But I need probably two weeks time because of other stuff.

blueyed added a commit to blueyed/pytest-django that referenced this issue Sep 16, 2018

Run tests in the same order as Django
This uses `pytest_collection_modifyitems` to order the tests.

Fixes pytest-dev#214.
@melinath

This comment has been minimized.

Copy link

melinath commented Nov 3, 2018

Just wanted to add a +1 to this; I ran into the ordering issue as well. Workaround I used was to add the following to conftest.py:

from django.test import TestCase
from django.test import TransactionTestCase


def pytest_collection_modifyitems(items):
    # Sort items to put transactional test cases at the end.
    items.sort(
        key=lambda item: int((
            issubclass(item.cls, TransactionTestCase)
            and not issubclass(item.cls, TestCase)
        ))
    )

Important thing to note is that TestCase is a subclass of TransactionTestCase.

It would be nice if pytest-django had this built in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment