From 5f20caa6e00d82d2f3fe9b7dc64d468e578c3d2c Mon Sep 17 00:00:00 2001
From: Wyatt Hoodes <hoodesw@hawaii.edu>
Date: Fri, 5 Jul 2019 09:50:51 -1000
Subject: [PATCH] test_upload: Refactor test_upload output to new filepath.

We write a function to set the `LOCAL_UPLOADS_DIR` path depending
on whether tests are being run in parallel or serial mode.
---
 zerver/lib/test_runner.py   | 32 ++++++++++++++++++++++----------
 zerver/tests/test_upload.py | 10 ++++++----
 zproject/test_settings.py   |  6 +-----
 3 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/zerver/lib/test_runner.py b/zerver/lib/test_runner.py
index 14241978cc..8085da8061 100644
--- a/zerver/lib/test_runner.py
+++ b/zerver/lib/test_runner.py
@@ -305,16 +305,7 @@ def init_worker(counter: Synchronized) -> None:
 
     destroy_test_databases(_worker_id)
     create_test_databases(_worker_id)
-
-    # Allow each child process to write to a unique directory within `TEST_RUN_DIR`.
-    worker_path = os.path.join(TEST_RUN_DIR, 'worker_{}'.format(_worker_id))
-    os.makedirs(worker_path, exist_ok=True)
-    settings.TEST_WORKER_DIR = worker_path
-
-    # Every process should upload to a separate directory so that
-    # race conditions can be avoided.
-    settings.LOCAL_UPLOADS_DIR = '{}_{}'.format(settings.LOCAL_UPLOADS_DIR,
-                                                _worker_id)
+    initialize_worker_path(_worker_id)
 
     def is_upload_avatar_url(url: RegexURLPattern) -> bool:
         if url.regex.pattern == r'^user_avatars/(?P<path>.*)$':
@@ -394,6 +385,22 @@ def check_import_error(test_name: str) -> None:
     except ImportError as exc:
         raise exc from exc  # Disable exception chaining in Python 3.
 
+
+def initialize_worker_path(worker_id: int) -> None:
+    # Allow each test worker process to write to a unique directory
+    # within `TEST_RUN_DIR`.
+    worker_path = os.path.join(TEST_RUN_DIR, 'worker_{}'.format(_worker_id))
+    os.makedirs(worker_path, exist_ok=True)
+    settings.TEST_WORKER_DIR = worker_path
+
+    # Every process should upload to a separate directory so that
+    # race conditions can be avoided.
+    settings.LOCAL_UPLOADS_DIR = get_or_create_dev_uuid_var_path(
+        os.path.join("test-backend",
+                     os.path.basename(TEST_RUN_DIR),
+                     os.path.basename(worker_path),
+                     "test_uploads"))
+
 class Runner(DiscoverRunner):
     test_suite = TestSuite
     test_loader = TestLoader()
@@ -444,6 +451,11 @@ class Runner(DiscoverRunner):
             else:
                 f.write(str(random_id_range_start) + "\n")
 
+        # Check if we are in serial mode to avoid unnecessarily making a directory.
+        # We add "worker_0" in the path for consistency with parallel mode.
+        if self.parallel == 1:
+            initialize_worker_path(0)
+
         return super().setup_test_environment(*args, **kwargs)
 
     def teardown_test_environment(self, *args: Any, **kwargs: Any) -> Any:
diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py
index fef78078d4..f6126c65fe 100644
--- a/zerver/tests/test_upload.py
+++ b/zerver/tests/test_upload.py
@@ -42,7 +42,8 @@ from zerver.lib.cache import get_realm_used_upload_space_cache_key, cache_get
 from zerver.lib.create_user import copy_user_settings
 from zerver.lib.users import get_api_key
 
-from scripts.lib.zulip_tools import get_or_create_dev_uuid_var_path
+from scripts.lib.zulip_tools import get_or_create_dev_uuid_var_path, \
+    get_dev_uuid_var_path
 
 import urllib
 import ujson
@@ -691,11 +692,12 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
                 response = self.client_get(uri)
                 _get_sendfile.clear()
                 test_upload_dir = os.path.split(settings.LOCAL_UPLOADS_DIR)[1]
-                uuid_directory, test_backend = os.path.split(os.path.dirname(settings.LOCAL_UPLOADS_DIR))
+                test_run, worker = os.path.split(os.path.dirname(settings.LOCAL_UPLOADS_DIR))
                 self.assertEqual(response['X-Accel-Redirect'],
                                  '/serve_uploads/../../' +
-                                 os.path.join(os.path.basename(uuid_directory), test_backend) +
-                                 '/' + test_upload_dir +
+                                 os.path.basename(get_dev_uuid_var_path()) +
+                                 '/test-backend/' + os.path.basename(test_run) +
+                                 '/' + worker + '/' + test_upload_dir +
                                  '/files/' + fp_path + '/' + name_str_for_test)
                 if content_disposition != '':
                     self.assertIn('attachment;', response['Content-disposition'])
diff --git a/zproject/test_settings.py b/zproject/test_settings.py
index 2e732abb9d..f1e13364c4 100644
--- a/zproject/test_settings.py
+++ b/zproject/test_settings.py
@@ -18,8 +18,6 @@ if os.getenv("EXTERNAL_HOST") is None:
     os.environ["EXTERNAL_HOST"] = "testserver"
 from .settings import *
 
-from scripts.lib.zulip_tools import get_or_create_dev_uuid_var_path
-
 # Clear out the REALM_HOSTS set in dev_settings.py
 REALM_HOSTS = {}
 
@@ -126,11 +124,9 @@ if not CASPER_TESTS:
 # Enable file:/// hyperlink support by default in tests
 ENABLE_FILE_LINKS = True
 
-
-LOCAL_UPLOADS_DIR = get_or_create_dev_uuid_var_path('test-backend/test_uploads')
-
 # These settings are set dynamically in `zerver/lib/test_runner.py`:
 TEST_WORKER_DIR = ''
+LOCAL_UPLOADS_DIR = ''
 
 S3_KEY = 'test-key'
 S3_SECRET_KEY = 'test-secret-key'