Use a temporary folder to store uploaded files during tests

[Shai Berger]

Today, when running tests, Django uses the production storage for uploaded files -- meaning any tests which upload files will save copies of them, under different names, for every test run.

We need to treat this essentially like we treat mail -- during tests, a special file-storage should be set up to receive the uploads. Like with mail, it is probably better for this folder to be kept after the test run ends, and be cleared only when the tests are run again; but this is of lower priority.

This should be the default, or enabled easily in settings.

As @apollo13 noted, Django's own tests define an environment variable:

TEMP_DIR = tempfile.mkdtemp(prefix='django_')
os.environ['DJANGO_TEST_TEMP_DIR'] = TEMP_DIR

and all storages used in the suite use it or a subfolder. This alone, however, is not enough for user tests, as there is currently no way to define separate storages for test and production.

[Berker Peksag]

Pavel, do you working on this? If not, I can take a look at this.

[Pavel Shpilev]

Oops, sorry. I was sure I deassigned myself.
Please, feel free to take it over.

[Carl Meyer]

FWIW, there's also ​django-inmemorystorage, which is even simpler/faster. But it wouldn't allow leaving the files around after a failed test run for inspection.

[Marc Tamlyn]

As discussed ​on the mailing list this links nicely to the idea of a STORAGES setting, and a new temp storage backend for testing purposes like some of our other dummy back ends.

I think it also relates to ideas in #24721 about test extensions, so that the temo storage could (optionally?) be cleared in teardown/teardowntestdata or similar.

[Sasha Gaevsky]

Replying to mjtamlyn:

As discussed on the mailing list[1] this links nicely to the idea of a STORAGES setting, and a new temp storage backend for testing purposes like some of our other dummy back ends.

Link is pointing to changeset, not mailing list. Could you please update it? Thanks.

[Tim Graham]

I edited comment 6 to add the link.

[Sasha Gaevsky]

Replying to timgraham:

I edited comment 6 to add the link.

Thanks, I've reviewed.

So basically the idea is to introduce STORAGES setting, move there DEFAULT_FILE_STORAGE and STATICFILES_STORAGE settings and finally introduce separate storage backend for testing?

[Aymeric Augustin]

I created #26029 to track the concept of multiple file storage backends, which may indeed make this easier to implement.

[אורי]

In Speedy Net I defined TESTS_MEDIA_ROOT in settings, which is different than the production MEDIA_ROOT (it is defined only when running tests). All the files in the tests are saved there and it is deleted after the tests end. We might want to do something similar in Django for all users, and maybe define a default value to TESTS_MEDIA_ROOT.

I also defined a variable TESTS in settings, which is True only when running tests, and the tests asserts this value to True before running tests. This is for not to run tests accidentally with the production or other settings, but only with the tests settings. We might want to add a similar variable to the settings in Django.

You can see my commits (from today) in this branch:
​https://github.com/speedy-net/speedy-net/commits/uri_main_branch_2019-06-30_a

[Bhavya Peshavaria]

I like the idea of using TESTS_MEDIA_ROOT as suggested by אורי. However, how would we handle it when the user passes a custom Storage object rather than using DEFAULT_FILE_STORAGE?

My current plan is to keep the TESTS_MEDIA_ROOT optional in Django settings. If it is defined, then the files will be uploaded to that path. If not, then will follow default behaviour to maintain reverse compatibility.

[Bhavya Peshavaria]

PR:[ ​https://github.com/django/django/pull/15848]

[Shai Berger]

In the years since this ticket was first filed, cloud services have become even more pervasive and important than they were. IMO, this means that the TEST_MEDIA_ROOT approach is inferior to a TEST_FILE_STORAGE, where the whole default storage is replaced for tests. That would allow users to choose if the media in their tests are saved to memory (as suggested in comment:5), to a different bucket in their cloud object storage, or to a local file. Of course, if a user explicitly creates their own storage in code, there's little Django can do as a framework.

Also, I don't think backwards compatibility in the sense of "keep putting test files into production folders" is something we want here. I'd much prefer things working properly out of the box -- meaning, the default should be that media files go into a temporary folder on the local system.

[Mariusz Felisiak]

Replying to Shai Berger:

In the years since this ticket was first filed, cloud services have become even more pervasive and important than they were. IMO, this means that the TEST_MEDIA_ROOT approach is inferior to a TEST_FILE_STORAGE, where the whole default storage is replaced for tests. That would allow users to choose if the media in their tests are saved to memory (as suggested in comment:5), to a different bucket in their cloud object storage, or to a local file. Of course, if a user explicitly creates their own storage in code, there's little Django can do as a framework.

Agreed, we should first resolved #26029 (​PR) to add STORAGES, then we can add support for the new test key.

[Jarosław Wygoda]

Hi Bhavya! Are you working on this ticket? Can I take it over?

[Bhavya Peshavaria]

Replying to Jarosław Wygoda:

Hi Bhavya! Are you working on this ticket? Can I take it over?

I apologize for the delay, but I am currently working on this ticket. It's taking a bit longer than expected.

[Bhavya Peshavaria]

Hi Jarosław Wygoda! I am deassigning from this ticket, you could take it over if needed.

[bcail]

Hi Jarosław, are you planning to work on this ticket? If not, I can work on it.

For everyone, is the plan here to check for a "test" key in STORAGES, and use that if it's present, or else use a custom temporary directory that gets automatically created and destroyed?

[bcail]

Here's a small patch that sets the default storage to a temporary directory:

diff --git a/django/test/utils.py b/django/test/utils.py
index ddb85127dc..2564e745ca 100644
--- a/django/test/utils.py
+++ b/django/test/utils.py
@@ -1,9 +1,11 @@
 import collections
+import copy
 import gc
 import logging
 import os
 import re
 import sys
+import tempfile
 import time
 import warnings
 from contextlib import contextmanager
@@ -149,6 +151,10 @@ def setup_test_environment(debug=None):
     saved_data.template_render = Template._render
     Template._render = instrumented_test_render
 
+    saved_data.storages = copy.deepcopy(settings.STORAGES)
+    settings.STORAGES["default"]["BACKEND"] = "django.core.files.storage.FileSystemStorage"
+    settings.STORAGES["default"]["OPTIONS"] = {"location": tempfile.mkdtemp()}
+
     mail.outbox = []
 
     deactivate()
@@ -165,6 +171,7 @@ def teardown_test_environment():
     settings.DEBUG = saved_data.debug
     settings.EMAIL_BACKEND = saved_data.email_backend
     Template._render = saved_data.template_render
+    settings.STORAGES = saved_data.storages
 
     del _TestState.saved_data
     del mail.outbox

Note: it doesn't currently allow for users to opt out of that behavior. Also, it makes some of the tests fail when they expect the settings to be different.

[bcail]

I opened a ​PR.

I fixed some of the tests by overriding the settings to be more like the defaults. There's one test that tests the defaults, and it's still failing.

Right now there's no override for the temp directory... do we want to add a setting? Or check for something in the STORAGES setting?