fix subtle gcp bug, move gcp asserts to tests

JJ11teen · May 16, 2021 · f1bc2b8 · f1bc2b8
1 parent 4fa6632
commit f1bc2b8
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 7 deletions.
diff --git a/src/cloudmappings/storageproviders/googlecloudstorage.py b/src/cloudmappings/storageproviders/googlecloudstorage.py
@@ -52,7 +52,8 @@ def upload_data(self, key: str, etag: str, data: bytes) -> str:
         b = self._bucket.get_blob(
             blob_name=key,
         )
-        if b is not None and (etag is None or etag != b.md5_hash):
+        existing_etag = None if b is None else b.md5_hash
+        if etag != existing_etag:
             self.raise_key_sync_error(key=key, etag=etag)
         if b is None:
             b = self._bucket.blob(
@@ -62,7 +63,6 @@ def upload_data(self, key: str, etag: str, data: bytes) -> str:
             data=data,
             if_generation_match=b.generation,
         )
-        assert b.md5_hash is not None
         return b.md5_hash
 
     def delete_data(self, key: str, etag: str) -> None:
@@ -84,6 +84,4 @@ def list_keys_and_etags(self, key_prefix: str) -> Dict[str, str]:
                 prefix=key_prefix,
             )
         }
-        for md5 in keys_and_ids.values():
-            assert md5 is not None
         return keys_and_ids
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -38,8 +38,8 @@ def pytest_addoption(parser):
 
 def pytest_configure(config):
     test_container_name = f"pytest-{config.getoption('test_container_id')}"
-    logging.info(f"Using cloud containers with the name: {test_container_name}")
-    logging.info(f"Using keys with the prefix: {test_run_id}")
+    logging.warning(f"Using cloud containers with the name: {test_container_name}")
+    logging.warning(f"Using keys with the prefix: {test_run_id}")
 
     azure_storage_account_url = config.getoption("azure_storage_account_url")
     if azure_storage_account_url is not None:

diff --git a/tests/tests/1_storageproviders.py b/tests/tests/1_storageproviders.py
@@ -33,6 +33,9 @@ def test_keys_and_etags_are_listed(self, storage_provider, test_id):
         assert etag_1 == keys_and_etags[key_1]
         assert etag_2 == keys_and_etags[key_2]
 
+        for key, etag in keys_and_etags.items():
+            assert etag is not None, (key, etag)
+
     def test_keys_are_deleted(self, storage_provider, test_id):
         key = test_id + "-keys-deleted-test"
 
@@ -48,7 +51,9 @@ def test_etags_are_enforced(self, storage_provider, test_id):
         with pytest.raises(KeySyncError):
             storage_provider.upload_data(key, "etag-when-none-existing", b"data")
 
-        storage_provider.upload_data(key, None, b"0")
+        good_etag = storage_provider.upload_data(key, None, b"0")
+        assert good_etag is not None
+
         with pytest.raises(KeySyncError):
             storage_provider.download_data(key, "bad-etag")
         with pytest.raises(KeySyncError):