-
Notifications
You must be signed in to change notification settings - Fork 17
Populate development dbs with test fixture data #2430
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
Populate development dbs with test fixture data #2430
Conversation
Jongmassey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having some trouble getting this running on my machine, I'll do a bit more investigating to see why that might be.
9bd7660 to
c4417ca
Compare
6473cfc to
9bc2ecf
Compare
Jongmassey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to approve this because it works as far as I can tell.
However, the use of the function in_production() to return the state of a django setting seems odd to me. I've made a suggestion as to how I might do it differently, but if there were good reasons for doing it your way please do shout!
| build-dbs-for-local-development`""" | ||
|
|
||
| def handle(self, *args, **options): | ||
| if in_production(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any need for this function rather than just a straight call to settings?
| if in_production(): | |
| if not settings.DEBUG: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patching the DEBUG env var and the DEBUG setting in settings.py were both tried and unfortunately didn't work. I don't have definitive proof, but from what I can tell this is due to the way the OpenCodelists app is initialized during testing: App set up seems to happen very early, i.e. right at the start of a test session. This would mean Django has already initialized the app, including reading in the env vars and settings module, before any tests run. If I change a setting in the settings module during a test, the app isn’t reinitialized to pick up that change, so the original setting e.g., DEBUG=True, persists.
To try and work around this, I created a separate function—in_production()—which reads the current value of the DEBUG setting, and can be monkeypatched during testing to simulate different environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest-django provides a fixture to override settings. Docs. I think that would be the more idiomatic approach.
|
|
||
|
|
||
| def test_setup_local_dev_databases_exits_early_when_in_prod_environment(monkeypatch): | ||
| monkeypatch.setattr(setup_local_dev_databases, "in_production", lambda: True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A purist might say this doesn't quite test whether this would run in production or not. If there were some bug in in_production()'s interpretation of django settings then this test could give an erroneously false pass
| from opencodelists.management.commands import setup_local_dev_databases | ||
|
|
||
|
|
||
| def test_setup_local_dev_databases_exits_early_when_in_prod_environment(monkeypatch): | ||
| monkeypatch.setattr(setup_local_dev_databases, "in_production", lambda: True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| from opencodelists.management.commands import setup_local_dev_databases | |
| def test_setup_local_dev_databases_exits_early_when_in_prod_environment(monkeypatch): | |
| monkeypatch.setattr(setup_local_dev_databases, "in_production", lambda: True) | |
| from opencodelists import settings | |
| def test_setup_local_dev_databases_exits_early_when_in_prod_environment(monkeypatch): | |
| monkeypatch.setattr(settings, "DEBUG", False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest-django provides a fixture to override settings. Docs. I think that would be the more idiomatic approach.
Closes #2313. It was suggested in #2313 that using existing test fixture data as dummy data for local development purposes may provide a good alternative to using a copy of prod databases. The approach taken here shows one way to populate local dbs with test fixture data, based on a comment on the original issue, and a spike (#2407). Co-authored-by: Jon Massey <jon.massey@thedatalab.org>
9bc2ecf to
7ab10ad
Compare
Jongmassey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send it!
Closes #2313.
What has been done?
A
setup_local_dev_databasesmanagement command deletes, rebuilds and populates databases (dbs). This is called from ajustrecipe which creates a new empty local core db and applies migrations. Both are safe by default. The result is a lightweight local development setup that uses our test data.You can use the
justrecipe in safe-mode:just build-dbs-for-local-developmentTo actually create a new empty local core db if you already have one, pass the
nuclearargument:just build-dbs-for-local-development nuclearA note on design choices
Originally, deletion of the core db occurred inside the management command. However, this led to unexpected behaviour. When a Django management command is invoked, it triggers
django.setup()and the app initialization process. During initialization, Django loadssettings.pyand expects the core database to exist at the path inDATABASES. If it's missing, SQLite creates adb.sqlite3file automatically. If the file is deleted after initialization, when migrations are run later in the same process, they don’t trigger a reinitialization of the app and re-check for the presence of the database file, so a new core db isn't recreated.Addressing this would require reinitializing the Django app from within the command — a potentially complex rabbit hole. Instead, the deletion step was moved out of the management command and into the
justrecipe.