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

speed up CI by running each test app separately #179

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

timgraham
Copy link
Collaborator

@timgraham timgraham commented Oct 31, 2024

Since django-mongodb does not support transactions, the database must be flushed between tests which means a collection.delete_many({}) for every collection that exists. By running each test app separately, it avoids the need to flush the collections in other apps.

Current test suite run time decreased from ~100 minutes to ~8 minutes.

A downside to this approach is that test failures won't be summarized at the bottom of the logs. You'll have to search for "FAIL:", "ERROR:", or "unexpected success" to find what went wrong.

Copy link
Collaborator

@aclark4life aclark4life left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Jibola
Copy link
Collaborator

Jibola commented Oct 31, 2024

Since django-mongodb does not support transactions, the database must be flushed between tests which means a collection.delete_many({}) for every collection that exists. By running each test app separately, it avoids the need to flush the collections in other apps.

Current test suite run time decreased from ~100 minutes to ~8 minutes.

A downside to this approach is that test failures won't be summarized at the bottom of the logs. You'll have to search for "FAIL:", "ERROR:", or "unexpected success" to find what went wrong.

Is it at all possible for us to just drop the collection? This usually is much faster than processing individual deletes.

@timgraham
Copy link
Collaborator Author

That was the approach before 8b727e9, but some tests expect to be able to introspect the presence of a collection. Even if Django provided a hook to re-create collections (along with all indexes, etc.) before each test, I don't know if that approach would perform much better. (If we take that approach without this change, it amounts to essentially dropping the database and re-creating it between tests).

@Jibola
Copy link
Collaborator

Jibola commented Oct 31, 2024

That was the approach before 8b727e9, but some tests expect to be able to introspect the presence of a collection. Even if Django provided a hook to re-create collections (along with all indexes, etc.) before each test, I don't know if that approach would perform much better. (If we take that approach without this change, it amounts to essentially dropping the database and re-creating it between tests).

It should be pretty fast. I imagine these tests aren't populating the local instance with much data, and getting the index warm, as well is pretty quick.

If there's not an easily leveraged setup hook already. I'm fine with this as a stop-gap solution. We'd just need a JIRA or Issue ticket raised to keep track of coming back to this. Delete many calls can definitely become cumbersome.

Since django-mongodb does not support transactions, the database must be
flushed between tests which means a collection.delete_many({}) for every
collection that exists. By running each test app separately, it avoids the
need to flush the collections in other apps.
@timgraham timgraham force-pushed the runtests-separately branch from 65333f1 to 86d5885 Compare November 1, 2024 20:40
@timgraham timgraham merged commit 86d5885 into mongodb-labs:main Nov 1, 2024
4 checks passed
@timgraham timgraham deleted the runtests-separately branch December 7, 2024 00:29
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.

3 participants