Opened 3 years ago

Closed 2 years ago

Last modified 23 months ago

#20550 closed New feature (fixed)

Add an option to preserve the database between test runs

Reported by: Aymeric Augustin Owned by: Greg Chapple
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: gregchapple1@…, mmanfre@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Creating database tables is expensive. #20483 says it takes 200s to create the tables required by Django's test suite with PostgreSQL. At $DAY_JOB, we have a few hundreds of models, and it takes about one minute to create a test database with PostgreSQL and a SSD.

To mitigate this problem, I've implemented a custom test runner that adds --nocreatedb and --nodropdb options to django-admin.py test. The names are self-explanatory. I'm not going to publish the implementation because it's totally unsuitable for Django. (It monkey-patches BaseDatabaseCreation.create_test_db/destroy_test_db when the flags are set.)

I'd like to implement a similar feature in Django. In the light of my first experience, I suggest a simpler API. I would add a single flag --keepdb. When this flag is set:

  • at the beginning of the tests, if the database already exists, Django doesn't ask to delete it; it goes ahead and reuses it;
  • at the end of the tests, Django doesn't drop the database.

Change History (25)

comment:1 Changed 3 years ago by Tomek Paczkowski

I see another benefit of that: At {{ day_job }} we have database that can't be easily recreated by syncdb (it is used by other apps, too). Moreover, due to permissions we can't create and drop db on CI server. We currently we override setup_database and teardown_database to disable creation and deletion of test db altogether.

comment:2 Changed 3 years ago by Anssi Kääriäinen

Triage Stage: UnreviewedAccepted

I have the same use case as oinopion.

I wonder if there should be some safety net, that is a check that the used database is infact a test database, and that it contains at least somewhat up-to-date schema. The first is somewhat easy, create extra table on test database setup, say 'django_test_meta' or something like that. If it exists, then the DB is assumed to be a test database.

The second issue is a lot harder. It would be extremely convenient if the test runner could detect stale databases and recreate the test database if needed. Something like storing md5 checksum of the model setup per app. A lighter check could be to check that all tables for managed models exists, and maybe that all columns (without datatype check) exists, too.

Both of the additions are in the category "nice to have", not blockers for commit.

comment:3 Changed 3 years ago by Greg Chapple

Owner: changed from nobody to Greg Chapple
Status: newassigned

comment:4 Changed 3 years ago by Aymeric Augustin

I just found my original implementation for this feature: https://gist.github.com/aaugustin/3012318

As said in the original description, I think we can improve the API.

comment:5 Changed 2 years ago by Greg Chapple

aaugustin, thanks for this. I haven't had a huge amount of time to work on this lately, just getting bits here and there. This is very helpful though, thanks!

comment:6 Changed 2 years ago by Greg Chapple

Has patch: set
Patch needs improvement: set

I've created a pull request for this: https://github.com/django/django/pull/2726

There is one failing test, however, and I'm not exactly sure why it is failing. If anyone can give some feedback / insight on it that would be great!

Thanks!

comment:7 Changed 2 years ago by Greg Chapple

Patch needs improvement: unset

Ignore previous comment. I figured out why it was failing. I've updated the pull request, all tests passing.

comment:8 Changed 2 years ago by Aymeric Augustin

Owner: changed from Greg Chapple to Aymeric Augustin

Cool, I'll review your patch.

comment:9 Changed 2 years ago by Greg Chapple

Sweet, thanks.

comment:10 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

LGTM; will let Aymeric do a final review.

comment:11 Changed 2 years ago by Greg Chapple

Cc: gregchapple1@… added

comment:12 Changed 2 years ago by Greg Chapple <gregchapple1@…>

Resolution: fixed
Status: assignedclosed

In b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae:

Fixed #20550 -- Added ability to preserve test db between runs

comment:13 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

In 2a5c750ad151c51d875d41eb080e4c160e8a2627:

Merge pull request #2726 from gchp/ticket-20550

Fixed #20550 -- Added ability to preserve test db between runs

comment:14 Changed 2 years ago by Greg Chapple <gregchapple1@…>

In b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae:

Fixed #20550 -- Added ability to preserve test db between runs

comment:15 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

In 2a5c750ad151c51d875d41eb080e4c160e8a2627:

Merge pull request #2726 from gchp/ticket-20550

Fixed #20550 -- Added ability to preserve test db between runs

comment:16 Changed 2 years ago by Aymeric Augustin

Greg, next time you submit a patch, please add yourself to AUTHORS! (Sorry, I never remember to check that before merging patches.)

comment:17 Changed 2 years ago by Greg Chapple

Ah, I always forget about that. I'll try remember next time. Thanks!

comment:18 Changed 2 years ago by Greg Chapple

Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Re-opening this per a discussion on IRC where it was highlighted that BaseDatabaseCreation.create_test_db receives the new keepdb option, but BaseDatabaseCreation.destroy_test_db (and subclasses) do not, which leaves a strange asymmetry.

I'm putting together a patch for this now.

comment:19 Changed 2 years ago by Greg Chapple

Owner: changed from Aymeric Augustin to Greg Chapple
Status: newassigned

comment:20 Changed 2 years ago by Michael Manfre

Cc: mmanfre@… added

comment:21 Changed 2 years ago by Greg Chapple

comment:22 Changed 2 years ago by Greg Chapple <gregchapple1@…>

Resolution: fixed
Status: assignedclosed

In 72f055e535fb25f8b18be031f8fe18674c1d7f6a:

Fixed #20550 -- Added keepdb argument to destroy_test_db

comment:23 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

In 1181b8a4a95551659be6737e41e9fc9d23eb8df4:

Merge pull request #2764 from gchp/ticket-20550

Fixed #20550 -- Added keepdb argument to destroy_test_db

comment:24 Changed 2 years ago by Tim Graham <timograham@…>

In e645b79ef5e5238c418df8f4ab5d52eb7bbc78e7:

Added missing docs to DiscoverRunner for keepdb option; refs #20550.

comment:25 Changed 23 months ago by Tim Graham <timograham@…>

In 8330b50c8587fc5484650f5bf47a3f03b50fb2ca:

Fixed #23863 -- Made runtests accept the keepdb option.

refs #20550

Note: See TracTickets for help on using tickets.
Back to Top