Opened 22 months ago

Closed 10 months ago

Last modified 4 months ago

#20550 closed New feature (fixed)

Add an option to preserve the database between test runs

Reported by: aaugustin Owned by: gchp
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 22 months ago by oinopion

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 22 months ago by akaariai

  • Triage Stage changed from Unreviewed to Accepted

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 13 months ago by gchp

  • Owner changed from nobody to gchp
  • Status changed from new to assigned

comment:4 Changed 12 months ago by aaugustin

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 12 months ago by gchp

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 10 months ago by gchp

  • 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 10 months ago by gchp

  • 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 10 months ago by aaugustin

  • Owner changed from gchp to aaugustin

Cool, I'll review your patch.

comment:9 Changed 10 months ago by gchp

Sweet, thanks.

comment:10 Changed 10 months ago by timo

  • Triage Stage changed from Accepted to Ready for checkin

LGTM; will let Aymeric do a final review.

comment:11 Changed 10 months ago by gchp

  • Cc gregchapple1@… added

comment:12 Changed 10 months ago by Greg Chapple <gregchapple1@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae:

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

comment:13 Changed 10 months 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 10 months ago by Greg Chapple <gregchapple1@…>

In b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae:

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

comment:15 Changed 10 months 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 10 months ago by aaugustin

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 10 months ago by gchp

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

comment:18 Changed 10 months ago by gchp

  • Resolution fixed deleted
  • Status changed from closed to new
  • Triage Stage changed from Ready for checkin to Accepted

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 10 months ago by gchp

  • Owner changed from aaugustin to gchp
  • Status changed from new to assigned

comment:20 Changed 10 months ago by manfre

  • Cc mmanfre@… added

comment:22 Changed 10 months ago by Greg Chapple <gregchapple1@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 72f055e535fb25f8b18be031f8fe18674c1d7f6a:

Fixed #20550 -- Added keepdb argument to destroy_test_db

comment:23 Changed 10 months 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 5 months ago by Tim Graham <timograham@…>

In e645b79ef5e5238c418df8f4ab5d52eb7bbc78e7:

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

comment:25 Changed 4 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