Repository: mapillary/mapillary_tools

Issue:
tools 0.8.0 upload images over and over again
### Basic information
* Release version: `0.8.0` 
* System: `Linux` 
* Capture Device: `Action Camera` 

It seems that with 0.8.0 the tools don't know which pictures have already been uploaded, and starts all over again. I uploaded 2597 pictures yesterday, and I was astonished to find that one sequence shows double in the feed. I guess I uploaded this the day before...Today, I wanted to know if some pics are still to upload, and: it starts again from sequence 1 of 6.  
Command is:
```
ted@ted-Latitude-E5470:~$ mapillary_tools upload "Aayi/521MEDIA/"
```
Please check. I guess I'm just uploading the same first sequence the third time.

Discussion:

Patch:
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
index 1e05323..0d903f9 100644
--- a/.github/workflows/python-package.yml
+++ b/.github/workflows/python-package.yml
@@ -31,6 +31,13 @@ jobs:
       uses: actions/setup-python@v2
       with:
         python-version: ${{ matrix.python-version }}
+      if: matrix.platform != 'windows-latest'
+    - name: Set up 32bit Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+        architecture: 'x86'
+      if: matrix.platform == 'windows-latest'
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
diff --git a/README.md b/README.md
index 2c65efa..7feb466 100644
--- a/README.md
+++ b/README.md
@@ -394,11 +394,11 @@ mapillary_tools upload_zip "path/to/zipped_images/"
 class Uploader:
     def __init__(self, user_items: UserItem, emitter: EventEmitter = None, dry_run=False): ...
 
-    def upload_zipfile(self, zip_path: str) -> Optional[int]: ...
+    def upload_zipfile(self, zip_path: str) -> Optional[str]: ...
 
-    def upload_blackvue(self, blackvue_path: str) -> Optional[int]: ...
+    def upload_blackvue(self, blackvue_path: str) -> Optional[str]: ...
 
-    def upload_images(self, descs: List[ImageDescriptionFile]) -> Dict[str, int]: ...
+    def upload_images(self, descs: List[ImageDescriptionFile]) -> Dict[str, str]: ...
 ```
 
 #### Examples
diff --git a/mapillary_tools/constants.py b/mapillary_tools/constants.py
index 699704e..47659bb 100644
--- a/mapillary_tools/constants.py
+++ b/mapillary_tools/constants.py
@@ -1,5 +1,7 @@
 import os
 
+import appdirs
+
 _ENV_PREFIX = "MAPILLARY_TOOLS_"
 
 ANSI_BOLD = "\033[1m"
@@ -16,3 +18,5 @@ IMAGE_DESCRIPTION_FILENAME = os.getenv(
 SAMPLED_VIDEO_FRAMES_FILENAME = os.getenv(
     _ENV_PREFIX + "SAMPLED_VIDEO_FRAMES_FILENAME", "mapillary_sampled_video_frames"
 )
+
+USER_DATA_DIR = appdirs.user_data_dir(appname="mapillary_tools", appauthor="Mapillary")
diff --git a/mapillary_tools/upload.py b/mapillary_tools/upload.py
index 8e59fc7..01d8d36 100644
--- a/mapillary_tools/upload.py
+++ b/mapillary_tools/upload.py
@@ -34,10 +34,16 @@ FileType = Literal["blackvue", "images", "zip"]
 
 LOG = logging.getLogger(__name__)
 MAPILLARY_DISABLE_API_LOGGING = os.getenv("MAPILLARY_DISABLE_API_LOGGING")
+MAPILLARY__ENABLE_UPLOAD_HISTORY_FOR_DRY_RUN = os.getenv(
+    "MAPILLARY__ENABLE_UPLOAD_HISTORY_FOR_DRY_RUN"
+)
+# Disable if it's set to empty
 MAPILLARY_UPLOAD_HISTORY_PATH = os.getenv(
     "MAPILLARY_UPLOAD_HISTORY_PATH",
-    # To enable it by default
-    # os.path.join(config.DEFAULT_MAPILLARY_FOLDER, "upload_history"),
+    os.path.join(
+        constants.USER_DATA_DIR,
+        "upload_history",
+    ),
 )
 
 
@@ -143,7 +149,7 @@ def _history_desc_path(md5sum: str) -> str:
 
 
 def is_uploaded(md5sum: str) -> bool:
-    if MAPILLARY_UPLOAD_HISTORY_PATH is None:
+    if not MAPILLARY_UPLOAD_HISTORY_PATH:
         return False
     return os.path.isfile(_history_desc_path(md5sum))
 
@@ -154,12 +160,12 @@ def write_history(
     summary: T.Dict,
     descs: T.Optional[T.List[types.ImageDescriptionFile]] = None,
 ) -> None:
-    if MAPILLARY_UPLOAD_HISTORY_PATH is None:
+    if not MAPILLARY_UPLOAD_HISTORY_PATH:
         return
     path = _history_desc_path(md5sum)
     LOG.debug(f"Writing upload history: {path}")
     os.makedirs(os.path.dirname(path), exist_ok=True)
-    history: T.Dict = {
+    history: T.Dict[str, T.Any] = {
         "params": params,
         "summary": summary,
     }
@@ -577,8 +583,12 @@ def upload(
 
     # Setup the emitter -- the order matters here
 
+    enable_history = MAPILLARY_UPLOAD_HISTORY_PATH and (
+        not dry_run or MAPILLARY__ENABLE_UPLOAD_HISTORY_FOR_DRY_RUN == "YES"
+    )
+
     # Put it first one to cancel early
-    if not dry_run:
+    if enable_history:
         _setup_cancel_due_to_duplication(emitter)
 
     # This one set up tdqm
@@ -613,7 +623,7 @@ def upload(
     else:
         descs = []
 
-    if not dry_run:
+    if enable_history:
         _setup_write_upload_history(emitter, params, descs)
 
     mly_uploader = uploader.Uploader(user_items, emitter=emitter, dry_run=dry_run)
diff --git a/mapillary_tools/upload_api_v4.py b/mapillary_tools/upload_api_v4.py
index c0acbee..ac626e1 100644
--- a/mapillary_tools/upload_api_v4.py
+++ b/mapillary_tools/upload_api_v4.py
@@ -145,7 +145,7 @@ class UploadService:
                 f"Upload server error: File handle not found in the upload response {resp.text}"
             )
 
-    def finish(self, file_handle: str) -> int:
+    def finish(self, file_handle: str) -> str:
         headers = {
             "Authorization": f"OAuth {self.user_access_token}",
         }
@@ -173,7 +173,7 @@ class UploadService:
                 f"Upload server error: failed to create the cluster {resp.text}"
             )
 
-        return T.cast(int, cluster_id)
+        return T.cast(str, cluster_id)
 
 
 import random
@@ -219,8 +219,8 @@ class FakeUploadService(UploadService):
                     callback(chunk, None)
         return self.session_key
 
-    def finish(self, _: str) -> int:
-        return 0
+    def finish(self, _: str) -> str:
+        return "0"
 
     def fetch_offset(self) -> int:
         if random.random() <= self._error_ratio:
diff --git a/mapillary_tools/uploader.py b/mapillary_tools/uploader.py
index b0437a9..87c8080 100644
--- a/mapillary_tools/uploader.py
+++ b/mapillary_tools/uploader.py
@@ -69,6 +69,9 @@ class Progress(types.TypedDict, total=False):
     # Path to the Zipfile/BlackVue
     import_path: str
 
+    # Cluster ID after finishing the upload
+    cluster_id: str
+
 
 class UploadCancelled(Exception):
     pass
@@ -112,12 +115,13 @@ class Uploader:
 
     def upload_zipfile(
         self, zip_path: str, event_payload: T.Optional[Progress] = None
-    ) -> T.Optional[int]:
+    ) -> T.Optional[str]:
         with zipfile.ZipFile(zip_path) as ziph:
             namelist = ziph.namelist()
-
-        if not namelist:
-            raise RuntimeError(f"The zip file {zip_path} is empty")
+            if not namelist:
+                LOG.warning(f"Skipping empty zipfile: %s", zip_path)
+                return None
+            upload_md5sum = _hash_zipfile(ziph)
 
         if event_payload is None:
             event_payload = {}
@@ -131,6 +135,7 @@ class Uploader:
             try:
                 return _upload_zipfile_fp(
                     fp,
+                    upload_md5sum,
                     len(namelist),
                     self.user_items,
                     event_payload=T.cast(
@@ -144,7 +149,7 @@ class Uploader:
 
     def upload_blackvue(
         self, blackvue_path: str, event_payload: T.Optional[Progress] = None
-    ) -> T.Optional[int]:
+    ) -> T.Optional[str]:
         try:
             return upload_blackvue(
                 blackvue_path,
@@ -158,10 +163,10 @@ class Uploader:
 
     def upload_images(
         self, descs: T.List[types.ImageDescriptionFile]
-    ) -> T.Dict[str, int]:
+    ) -> T.Dict[str, str]:
         _validate_descs(descs)
         sequences = _group_sequences_by_uuid(descs)
-        ret: T.Dict[str, int] = {}
+        ret: T.Dict[str, str] = {}
         for sequence_idx, (sequence_uuid, images) in enumerate(sequences.items()):
             event_payload: Progress = {
                 "sequence_idx": sequence_idx,
@@ -170,10 +175,11 @@ class Uploader:
                 "sequence_uuid": sequence_uuid,
             }
             with tempfile.NamedTemporaryFile() as fp:
-                _zip_sequence_fp(images, fp)
+                upload_md5sum = _zip_sequence_fp(images, fp)
                 try:
-                    cluster_id: T.Optional[int] = _upload_zipfile_fp(
+                    cluster_id: T.Optional[str] = _upload_zipfile_fp(
                         fp,
+                        upload_md5sum,
                         len(images),
                         self.user_items,
                         emitter=self.emitter,
@@ -218,16 +224,25 @@ def zip_images(
             zip_dir, f"mly_tools_{sequence_uuid}.{os.getpid()}.wip"
         )
         with open(zip_filename_wip, "wb") as fp:
-            _zip_sequence_fp(sequence, fp)
-        upload_md5sum = utils.file_md5sum(zip_filename_wip)
+            upload_md5sum = _zip_sequence_fp(sequence, fp)
         zip_filename = os.path.join(zip_dir, f"mly_tools_{upload_md5sum}.zip")
         os.rename(zip_filename_wip, zip_filename)
 
 
+# Instead of hashing the zip file content, we hash the filename list,
+# because the zip content could be changed due to EXIF change
+# (e.g. changes in MAPMetaTag in image description)
+def _hash_zipfile(ziph: zipfile.ZipFile) -> str:
+    # namelist is List[str]
+    namelist = ziph.namelist()
+    concat = "".join(os.path.splitext(os.path.basename(name))[0] for name in namelist)
+    return utils.md5sum_bytes(concat.encode("utf-8"))
+
+
 def _zip_sequence_fp(
     sequence: T.Dict[str, types.ImageDescriptionFile],
     fp: T.IO[bytes],
-) -> None:
+) -> str:
     descs = list(sequence.values())
     descs.sort(
         key=lambda desc: (
@@ -238,34 +253,36 @@ def _zip_sequence_fp(
     with zipfile.ZipFile(fp, "w", zipfile.ZIP_DEFLATED) as ziph:
         for desc in descs:
             edit = exif_write.ExifEdit(desc["filename"])
+            with open(desc["filename"], "rb") as fp:
+                md5sum = utils.md5sum_fp(fp)
             exif_desc = desc_file_to_exif(desc)
             edit.add_image_description(exif_desc)
             image_bytes = edit.dump_image_bytes()
             # To make sure the zip file deterministic, i.e. zip same files result in same content (same hashes),
             # we use md5 as the name, and an constant as the modification time
-            _, ext = os.path.split(desc["filename"])
-            arcname = f"{utils.md5sum_bytes(image_bytes)}{ext.lower()}"
+            _, ext = os.path.splitext(desc["filename"])
+            arcname = f"{md5sum}{ext.lower()}"
             zipinfo = zipfile.ZipInfo(arcname, date_time=(1980, 1, 1, 0, 0, 0))
             ziph.writestr(zipinfo, image_bytes)
 
+        return _hash_zipfile(ziph)
+
 
 def _upload_zipfile_fp(
     fp: T.IO[bytes],
+    upload_md5sum: str,
     image_count: int,
     user_items: types.UserItem,
     event_payload: T.Optional[Progress] = None,
     emitter: T.Optional[EventEmitter] = None,
     dry_run=False,
-) -> int:
+) -> str:
     if event_payload is None:
         event_payload = {
             "sequence_idx": 0,
             "total_sequence_count": 1,
         }
 
-    fp.seek(0, io.SEEK_SET)
-    upload_md5sum = utils.md5sum_fp(fp)
-
     fp.seek(0, io.SEEK_END)
     entity_size = fp.tell()
 
@@ -308,7 +325,7 @@ def upload_blackvue(
     event_payload: T.Optional[Progress] = None,
     emitter: EventEmitter = None,
     dry_run=False,
-) -> int:
+) -> str:
     jsonschema.validate(instance=user_items, schema=types.UserItemSchema)
 
     with open(blackvue_path, "rb") as fp:
@@ -391,7 +408,7 @@ def _upload_fp(
     chunk_size: int,
     event_payload: Progress = None,
     emitter: EventEmitter = None,
-) -> int:
+) -> str:
     retries = 0
 
     if event_payload is None:
@@ -462,6 +479,7 @@ def _upload_fp(
         raise upload_api_v4.wrap_http_exception(ex) from ex
 
     if emitter:
+        mutable_payload["cluster_id"] = cluster_id
         emitter.emit("upload_finished", mutable_payload)
 
     return cluster_id
diff --git a/mypy.ini b/mypy.ini
index 8e39ec7..5200cfd 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -9,9 +9,6 @@ ignore_missing_imports = True
 [mypy-pynmea2.*]
 ignore_missing_imports = True
 
-[mypy-tzwhere.*]
-ignore_missing_imports = True
-
 [mypy-gpxpy.*]
 ignore_missing_imports = True
 
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 38160d1..0d433f5 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,10 +1,9 @@
-Pillow>=9.0.0,<10.0.0; python_version >= '3.7'
-Pillow>=8.0.0,<9.0.0; python_version < '3.7'
+Pillow>=9.0.0,<10.0.0; python_version>='3.7'
+Pillow>=8.0.0,<9.0.0; python_version<'3.7'
 pytest
 black
 mypy
 pyinstaller
 flake8
-types-python-dateutil
-types-pytz
 types-requests
+types-appdirs
diff --git a/requirements.txt b/requirements.txt
index 89f722c..4028386 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,3 +1,4 @@
+appdirs>=1.4.4,<2.0.0
 exifread==2.3.2
 piexif==1.1.3
 gpxpy==1.4.2


Test patch:
diff --git a/tests/integration/test_gopro.py b/tests/integration/test_gopro.py
index 044a714..b0756c4 100644
--- a/tests/integration/test_gopro.py
+++ b/tests/integration/test_gopro.py
@@ -5,7 +5,7 @@ import subprocess
 import pytest
 import py.path
 
-from .test_process import setup_config, EXECUTABLE, is_ffmpeg_installed
+from .test_process import setup_config, setup_upload, EXECUTABLE, is_ffmpeg_installed
 
 
 IMPORT_PATH = "tests/integration/mapillary_tools_process_images_provider/gopro_data"
@@ -91,13 +91,12 @@ def setup_data(tmpdir: py.path.local):
 
 
 def test_process_gopro_hero8(
-    tmpdir: py.path.local, setup_data: py.path.local, setup_config: py.path.local
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_upload: py.path.local,
 ):
     if not is_ffmpeg_installed:
         pytest.skip("skip because ffmpeg not installed")
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     video_path = setup_data.join("hero8.mp4")
     x = subprocess.run(
         f"{EXECUTABLE} video_process  --geotag_source=gopro_videos {str(video_path)} --interpolation_use_gpx_start_time",
@@ -109,7 +108,6 @@ def test_process_gopro_hero8(
     )
     assert desc_path.exists()
     with open(desc_path) as fp:
-        # print(fp.read())
         descs = json.load(fp)
     for expected, actual in zip(expected_descs, descs):
         assert abs(expected["MAPLatitude"] - actual["MAPLatitude"]) < 0.0001
@@ -131,7 +129,4 @@ def test_process_gopro_hero8(
             < 0.0001
         )
         assert expected["filename"] == actual["filename"]
-        assert {
-            "strings": [{"key": "mapillary_tools_version", "value": "0.8.2"}]
-        } == actual["MAPMetaTags"]
         assert "MAPSequenceUUID" in actual
diff --git a/tests/integration/test_history.py b/tests/integration/test_history.py
new file mode 100644
index 0000000..63738d3
--- /dev/null
+++ b/tests/integration/test_history.py
@@ -0,0 +1,74 @@
+import os
+import subprocess
+
+import pytest
+import py.path
+
+from .test_process import EXECUTABLE, USERNAME, setup_data, setup_config, setup_upload
+
+
+@pytest.fixture
+def setup_history_path(tmpdir: py.path.local):
+    os.environ["MAPILLARY__ENABLE_UPLOAD_HISTORY_FOR_DRY_RUN"] = "YES"
+    history_path = tmpdir.mkdir("history")
+    os.environ["MAPILLARY_UPLOAD_HISTORY_PATH"] = str(history_path)
+    yield history_path
+    if tmpdir.check():
+        history_path.remove(ignore_errors=True)
+    del os.environ["MAPILLARY_UPLOAD_HISTORY_PATH"]
+    del os.environ["MAPILLARY__ENABLE_UPLOAD_HISTORY_FOR_DRY_RUN"]
+
+
+def test_upload_images(
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_history_path: py.path.local,
+    setup_upload: py.path.local,
+):
+    assert len(setup_upload.listdir()) == 0
+
+    x = subprocess.run(
+        f"{EXECUTABLE} process_and_upload {str(setup_data)} --dry_run --user_name={USERNAME}",
+        shell=True,
+    )
+    assert x.returncode == 0, x.stderr
+    assert 0 < len(setup_upload.listdir()), "should be uploaded for the first time"
+    for upload in setup_upload.listdir():
+        upload.remove()
+
+    x = subprocess.run(
+        f"{EXECUTABLE} process_and_upload {str(setup_data)} --dry_run --user_name={USERNAME}",
+        shell=True,
+    )
+    assert x.returncode == 0, x.stderr
+    assert (
+        len(setup_upload.listdir()) == 0
+    ), "should NOT upload because it is uploaded already"
+
+
+def test_upload_blackvue(
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_history_path: py.path.local,
+    setup_upload: py.path.local,
+):
+    assert len(setup_upload.listdir()) == 0
+
+    x = subprocess.run(
+        f"{EXECUTABLE} upload_blackvue {str(setup_data)} --dry_run --user_name={USERNAME}",
+        shell=True,
+    )
+    assert x.returncode == 0, x.stderr
+    assert len(setup_upload.listdir()) == 1, "should be uploaded for the first time"
+    for upload in setup_upload.listdir():
+        upload.remove()
+    assert len(setup_upload.listdir()) == 0
+
+    x = subprocess.run(
+        f"{EXECUTABLE} upload_blackvue {str(setup_data)} --dry_run --user_name={USERNAME}",
+        shell=True,
+    )
+    assert x.returncode == 0, x.stderr
+    assert (
+        len(setup_upload.listdir()) == 0
+    ), "should NOT upload because it is uploaded already"
diff --git a/tests/integration/test_process.py b/tests/integration/test_process.py
index 3937178..2982146 100644
--- a/tests/integration/test_process.py
+++ b/tests/integration/test_process.py
@@ -27,6 +27,7 @@ def setup_config(tmpdir: py.path.local):
     yield config_path
     if tmpdir.check():
         tmpdir.remove(ignore_errors=True)
+    del os.environ["MAPILLARY_CONFIG_PATH"]
 
 
 @pytest.fixture
@@ -39,6 +40,18 @@ def setup_data(tmpdir: py.path.local):
         tmpdir.remove(ignore_errors=True)
 
 
+@pytest.fixture
+def setup_upload(tmpdir: py.path.local):
+    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
+    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
+    os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"] = "YES"
+    yield upload_dir
+    if tmpdir.check():
+        tmpdir.remove(ignore_errors=True)
+    del os.environ["MAPILLARY_UPLOAD_PATH"]
+    del os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"]
+
+
 def test_basic():
     for option in ["--version", "--help"]:
         x = subprocess.run(f"{EXECUTABLE} {option}", shell=True)
@@ -103,11 +116,11 @@ def test_zip(tmpdir: py.path.local, setup_data: py.path.local):
 
 
 def test_upload_image_dir(
-    tmpdir: py.path.local, setup_config: py.path.local, setup_data: py.path.local
+    tmpdir: py.path.local,
+    setup_config: py.path.local,
+    setup_data: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     x = subprocess.run(
         f"{EXECUTABLE} process {setup_data}",
         shell=True,
@@ -117,17 +130,17 @@ def test_upload_image_dir(
         f"{EXECUTABLE} upload {setup_data} --dry_run --user_name={USERNAME}",
         shell=True,
     )
-    for file in upload_dir.listdir():
+    for file in setup_upload.listdir():
         validate_and_extract_zip(str(file))
     assert x.returncode == 0, x.stderr
 
 
 def test_upload_image_dir_twice(
-    tmpdir: py.path.local, setup_config: py.path.local, setup_data: py.path.local
+    tmpdir: py.path.local,
+    setup_config: py.path.local,
+    setup_data: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     x = subprocess.run(
         f"{EXECUTABLE} process {setup_data}",
         shell=True,
@@ -142,7 +155,7 @@ def test_upload_image_dir_twice(
         shell=True,
     )
     assert x.returncode == 0, x.stderr
-    for file in upload_dir.listdir():
+    for file in setup_upload.listdir():
         validate_and_extract_zip(str(file))
         md5sum_map[os.path.basename(file)] = file_md5sum(file)
 
@@ -152,19 +165,19 @@ def test_upload_image_dir_twice(
         shell=True,
     )
     assert x.returncode == 0, x.stderr
-    for file in upload_dir.listdir():
+    for file in setup_upload.listdir():
         validate_and_extract_zip(str(file))
         new_md5sum = file_md5sum(file)
         assert md5sum_map[os.path.basename(file)] == new_md5sum
-    assert len(md5sum_map) == len(upload_dir.listdir())
+    assert len(md5sum_map) == len(setup_upload.listdir())
 
 
 def test_upload_zip(
-    tmpdir: py.path.local, setup_data: py.path.local, setup_config: py.path.local
+    tmpdir: py.path.local,
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     zip_dir = tmpdir.mkdir("zip_dir")
     x = subprocess.run(
         f"{EXECUTABLE} process {setup_data}",
@@ -182,22 +195,22 @@ def test_upload_zip(
             shell=True,
         )
         assert x.returncode == 0, x.stderr
-    for file in upload_dir.listdir():
+    for file in setup_upload.listdir():
         validate_and_extract_zip(str(file))
 
 
 def test_process_and_upload(
-    tmpdir: py.path.local, setup_config: py.path.local, setup_data: py.path.local
+    tmpdir: py.path.local,
+    setup_config: py.path.local,
+    setup_data: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     x = subprocess.run(
         f"{EXECUTABLE} process_and_upload {setup_data} --dry_run --user_name={USERNAME}",
         shell=True,
     )
     assert x.returncode == 0, x.stderr
-    for file in upload_dir.listdir():
+    for file in setup_upload.listdir():
         validate_and_extract_zip(str(file))
 
 
@@ -323,7 +336,6 @@ def test_angle(setup_data: py.path.local):
 def test_process_boolean_options(
     setup_config: py.path.local, setup_data: py.path.local
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
     boolean_options = [
         "--add_file_name",
         "--add_import_date",
@@ -660,34 +672,31 @@ def file_md5sum(path) -> str:
 
 
 def test_upload_multiple_mp4s_DEPRECATED(
-    tmpdir: py.path.local, setup_data: py.path.local, setup_config: py.path.local
+    tmpdir: py.path.local,
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"] = "YES"
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
     video_path = setup_data.join("sample-5s.mp4")
     x = subprocess.run(
         f"{EXECUTABLE} upload {video_path} {video_path} --dry_run --user_name={USERNAME}",
         shell=True,
     )
 
-    assert 1 == len(upload_dir.listdir())
+    assert 1 == len(setup_upload.listdir())
     assert {"mly_tools_8cd0e9af15f4baaafe9dfe98ace8b886.mp4"} == {
-        os.path.basename(f) for f in upload_dir.listdir()
+        os.path.basename(f) for f in setup_upload.listdir()
     }
     md5sum = file_md5sum(video_path)
-    assert {md5sum} == {file_md5sum(f) for f in upload_dir.listdir()}
+    assert {md5sum} == {file_md5sum(f) for f in setup_upload.listdir()}
 
 
 def test_upload_blackvue(
-    tmpdir: py.path.local, setup_data: py.path.local, setup_config: py.path.local
+    tmpdir: py.path.local,
+    setup_data: py.path.local,
+    setup_config: py.path.local,
+    setup_upload: py.path.local,
 ):
-    os.environ["MAPILLARY_CONFIG_PATH"] = str(setup_config)
-    os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"] = "YES"
-    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
-
     another_path = tmpdir.join("another_sub")
 
     video_path2 = another_path.join("sub1 folder").join("sub2 folder").join("hello.mp4")
@@ -710,9 +719,9 @@ def test_upload_blackvue(
     )
     assert x.returncode == 0, x.stderr
 
-    assert 3 == len(upload_dir.listdir())
+    assert 3 == len(setup_upload.listdir())
     assert {
         "mly_tools_8cd0e9af15f4baaafe9dfe98ace8b886.mp4",
         f"mly_tools_{file_md5sum(str(video_path2))}.mp4",
         f"mly_tools_{file_md5sum(str(video_path_hello2))}.mp4",
-    } == {os.path.basename(f) for f in upload_dir.listdir()}
+    } == {os.path.basename(f) for f in setup_upload.listdir()}
diff --git a/tests/unit/test_uploader.py b/tests/unit/test_uploader.py
index b528cec..7f68371 100644
--- a/tests/unit/test_uploader.py
+++ b/tests/unit/test_uploader.py
@@ -4,8 +4,9 @@ import zipfile
 import tempfile
 
 import py.path
+import pytest
 
-from mapillary_tools import uploader, exif_read, utils
+from mapillary_tools import uploader, exif_read
 
 
 def _validate_and_extract_zip(filename: str):
@@ -30,15 +31,27 @@ def _validate_and_extract_zip(filename: str):
 
 def _validate_zip_dir(zip_dir: py.path.local):
     for zip_path in zip_dir.listdir():
-        upload_md5sum = utils.file_md5sum(str(zip_path))
+        with zipfile.ZipFile(zip_path) as ziph:
+            upload_md5sum = uploader._hash_zipfile(ziph)
         assert (
             str(os.path.basename(zip_path)) == f"mly_tools_{upload_md5sum}.zip"
         ), zip_path
         _validate_and_extract_zip(str(zip_path))
 
 
-def test_upload_images(tmpdir: py.path.local):
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(tmpdir.join("uploaded"))
+@pytest.fixture
+def setup_upload(tmpdir: py.path.local):
+    upload_dir = tmpdir.mkdir("mapillary_public_uploads")
+    os.environ["MAPILLARY_UPLOAD_PATH"] = str(upload_dir)
+    os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"] = "YES"
+    yield upload_dir
+    if tmpdir.check():
+        tmpdir.remove(ignore_errors=True)
+    del os.environ["MAPILLARY_UPLOAD_PATH"]
+    del os.environ["MAPILLARY__DISABLE_BLACKVUE_CHECK"]
+
+
+def test_upload_images(setup_upload: py.path.local):
     mly_uploader = uploader.Uploader(
         {"user_upload_token": "YOUR_USER_ACCESS_TOKEN"}, dry_run=True
     )
@@ -58,12 +71,11 @@ def test_upload_images(tmpdir: py.path.local):
     ]
     resp = mly_uploader.upload_images(descs)
     assert len(resp) == 1
-    assert len(tmpdir.join("uploaded").listdir()) == 1
-    _validate_zip_dir(tmpdir.join("uploaded"))
+    assert len(setup_upload.listdir()) == 1
+    _validate_zip_dir(setup_upload)
 
 
-def test_upload_images_multiple_sequences(tmpdir: py.path.local):
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(tmpdir.join("uploaded"))
+def test_upload_images_multiple_sequences(setup_upload: py.path.local):
     descs = [
         {
             "MAPLatitude": 58.5927694,
@@ -83,7 +95,7 @@ def test_upload_images_multiple_sequences(tmpdir: py.path.local):
             "MAPLatitude": 59.5927694,
             "MAPLongitude": 16.1840944,
             "MAPCaptureTime": "2021_02_13_13_25_41_140",
-            "filename": "tests/unit/data/test_exif.jpg",
+            "filename": "tests/unit/data/fixed_exif.jpg",
             "MAPSequenceUUID": "sequence_2",
         },
     ]
@@ -97,12 +109,11 @@ def test_upload_images_multiple_sequences(tmpdir: py.path.local):
     )
     resp = mly_uploader.upload_images(descs)
     assert len(resp) == 2
-    assert len(tmpdir.join("uploaded").listdir()) == 2
-    _validate_zip_dir(tmpdir.join("uploaded"))
+    assert len(setup_upload.listdir()) == 2
+    _validate_zip_dir(setup_upload)
 
 
-def test_upload_zip(tmpdir: py.path.local, emitter=None):
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(tmpdir.join("uploaded"))
+def test_upload_zip(tmpdir: py.path.local, setup_upload: py.path.local, emitter=None):
     same_basename = tmpdir.join("text_exif.jpg")
     py.path.local("tests/unit/data/test_exif.jpg").copy(tmpdir.join("text_exif.jpg"))
     descs = [
@@ -145,11 +156,10 @@ def test_upload_zip(tmpdir: py.path.local, emitter=None):
     for zip_path in zip_dir.listdir():
         resp = mly_uploader.upload_zipfile(str(zip_path))
 
-    _validate_zip_dir(tmpdir.join("uploaded"))
+    _validate_zip_dir(setup_upload)
 
 
-def test_upload_blackvue(tmpdir: py.path.local):
-    os.environ["MAPILLARY_UPLOAD_PATH"] = str(tmpdir.join("uploaded"))
+def test_upload_blackvue(tmpdir: py.path.local, setup_upload: py.path.local):
     mly_uploader = uploader.Uploader(
         {
             "user_upload_token": "YOUR_USER_ACCESS_TOKEN",
@@ -162,8 +172,8 @@ def test_upload_blackvue(tmpdir: py.path.local):
     with open(blackvue_path, "wb") as fp:
         fp.write(b"this is a fake video")
     resp = mly_uploader.upload_blackvue(str(blackvue_path))
-    assert resp == 0
-    for mp4_path in tmpdir.join("uploaded").listdir():
+    assert resp == "0"
+    for mp4_path in setup_upload.listdir():
         basename = os.path.basename(mp4_path)
         assert str(basename).startswith("mly_tools_")
         assert str(basename).endswith(".mp4")
@@ -171,7 +181,7 @@ def test_upload_blackvue(tmpdir: py.path.local):
             assert fp.read() == b"this is a fake video"
 
 
-def test_upload_zip_with_emitter(tmpdir: py.path.local):
+def test_upload_zip_with_emitter(tmpdir: py.path.local, setup_upload: py.path.local):
     emitter = uploader.EventEmitter()
 
     stats = {}
@@ -202,6 +212,6 @@ def test_upload_zip_with_emitter(tmpdir: py.path.local):
 
         assert payload["md5sum"] in stats
 
-    test_upload_zip(tmpdir, emitter=emitter)
+    test_upload_zip(tmpdir, setup_upload, emitter=emitter)
 
     assert len(stats) == 2