[Django] #20550: Add an option to preserve the database between test runs
Django
---------------------------------------------+------------------------
Reporter: aaugustin | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: aaugustin | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:1>
Django
Reporter: aaugustin | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akaariai):
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:2>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* status: new => assigned
* owner: nobody => gchp
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:3>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:4>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (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!
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:5>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:6>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* needs_better_patch: 1 => 0
Comment:
Ignore previous comment. I figured out why it was failing. I've updated
the pull request, all tests passing.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:7>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* owner: gchp => aaugustin
Comment:
Cool, I'll review your patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:8>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by gchp):
Sweet, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:9>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timo):
* stage: Accepted => Ready for checkin
Comment:
LGTM; will let Aymeric do a final review.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:10>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: assigned
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* cc: gregchapple1@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:11>
Django
Reporter: aaugustin | Owner: aaugustin
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"2a5c750ad151c51d875d41eb080e4c160e8a2627"]:
{{{
#!CommitTicketReference repository=""
revision="2a5c750ad151c51d875d41eb080e4c160e8a2627"
Merge pull request #2726 from gchp/ticket-20550
Fixed #20550 -- Added ability to preserve test db between runs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:13>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae"]:
{{{
#!CommitTicketReference repository=""
revision="b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae"
Fixed #20550 -- Added ability to preserve test db between runs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:12>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Greg Chapple <gregchapple1@…>):
In [changeset:"b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae"]:
{{{
#!CommitTicketReference repository=""
revision="b7aa7c4ab4372d2b7994d252c8bc87f77dd217ae"
Fixed #20550 -- Added ability to preserve test db between runs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:14>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"2a5c750ad151c51d875d41eb080e4c160e8a2627"]:
{{{
#!CommitTicketReference repository=""
revision="2a5c750ad151c51d875d41eb080e4c160e8a2627"
Merge pull request #2726 from gchp/ticket-20550
Fixed #20550 -- Added ability to preserve test db between runs
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:15>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Greg, next time you submit a patch, please add yourself to AUTHORS!
(Sorry, I never remember to check that before merging patches.)
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:16>
Django
Reporter: aaugustin | Owner: aaugustin
Type: New feature | Status: closed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by gchp):
Ah, I always forget about that. I'll try remember next time. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:17>
Django
Reporter: aaugustin | Owner: aaugustin
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* status: closed => new
* resolution: fixed =>
* stage: Ready for checkin => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:18>
Django
Reporter: aaugustin | Owner: gchp
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by gchp):
* owner: aaugustin => gchp
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:19>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by manfre):
* cc: mmanfre@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:20>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by gchp):
Created a PR here: https://github.com/django/django/pull/2764
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:21>
Django
Reporter: aaugustin | Owner: gchp
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Greg Chapple <gregchapple1@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"72f055e535fb25f8b18be031f8fe18674c1d7f6a"]:
{{{
#!CommitTicketReference repository=""
revision="72f055e535fb25f8b18be031f8fe18674c1d7f6a"
Fixed #20550 -- Added keepdb argument to destroy_test_db
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:22>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"1181b8a4a95551659be6737e41e9fc9d23eb8df4"]:
{{{
#!CommitTicketReference repository=""
revision="1181b8a4a95551659be6737e41e9fc9d23eb8df4"
Merge pull request #2764 from gchp/ticket-20550
Fixed #20550 -- Added keepdb argument to destroy_test_db
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:23>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e645b79ef5e5238c418df8f4ab5d52eb7bbc78e7"]:
{{{
#!CommitTicketReference repository=""
revision="e645b79ef5e5238c418df8f4ab5d52eb7bbc78e7"
Added missing docs to DiscoverRunner for keepdb option; refs #20550.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:24>
Django
Reporter: aaugustin | Owner: gchp
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8330b50c8587fc5484650f5bf47a3f03b50fb2ca"]:
{{{
#!CommitTicketReference repository=""
revision="8330b50c8587fc5484650f5bf47a3f03b50fb2ca"
Fixed #23863 -- Made runtests accept the keepdb option.
refs #20550
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20550#comment:25>