From 0e9265c22cc806ecf7ba7aad79bc7e41370cdf59 Mon Sep 17 00:00:00 2001 From: Constantine Nathanson Date: Sun, 4 Oct 2026 17:43:24 +0300 Subject: [PATCH] Speed up and restore `sync` and `upload_dir` integration tests - Wait for assets to be indexed instead of sleeping for fixed periods. `_wait_for_cld_files` polls Search until the folder has the expected number of assets, and fails with a clear message on timeout. - Run all `TestCLISync` tests again: `@retry_assertion` now works with and without parentheses and runs `tearDown` and `setUp` between tries. - Clean up dynamic folders with Search, and fall back to the Admin API when some assets are not indexed yet. - `sync` prints "Done!" after uploading or downloading files. - Update the expected log line for a missing local folder. Co-Authored-By: Claude Opus 5.5 (1M context) --- cloudinary_cli/modules/sync.py | 4 ++ test/helper_test.py | 70 +++++++++++++++++------- test/test_modules/test_cli_sync.py | 48 +++++----------- test/test_modules/test_cli_upload_dir.py | 3 - 4 files changed, 66 insertions(+), 59 deletions(-) diff --git a/cloudinary_cli/modules/sync.py b/cloudinary_cli/modules/sync.py index af3b107..a0402c6 100644 --- a/cloudinary_cli/modules/sync.py +++ b/cloudinary_cli/modules/sync.py @@ -208,6 +208,8 @@ def push(self): if upload_errors: raise Exception("Sync did not finish successfully") + return True + def pull(self): """ Pulls changes from the Cloudinary folder to the local folder. @@ -253,6 +255,8 @@ def pull(self): if download_errors: raise Exception("Sync did not finish successfully") + return True + def _normalize_remote_file_names(self, remote_files, local_files): """ When multiple remote files have duplicate display name, we save them locally by appending index at the end diff --git a/test/helper_test.py b/test/helper_test.py index a5d97fb..bac688b 100644 --- a/test/helper_test.py +++ b/test/helper_test.py @@ -95,32 +95,58 @@ def get_params(mocker): return params -def retry_assertion(num_tries=3, delay=3): +def retry_assertion(func=None, *, num_tries=3, delay=3): """ - Helper for retrying inconsistent unit tests + Helper for retrying inconsistent unit tests, running tearDown and setUp between tries + :param func: The test method, when used without parentheses :param num_tries: Number of tries to perform :param delay: Delay in seconds between retries """ + if func is None: + return lambda f: retry_assertion(f, num_tries=num_tries, delay=delay) - def retry_decorator(func): - @wraps(func) - def retry_func(*args, **kwargs): - try_num = 1 - while try_num < num_tries: - try: - return func(*args, **kwargs) - except AssertionError: - logger.warning("Assertion #{} out of {} failed, retrying in {} seconds".format(try_num, num_tries, - delay)) - time.sleep(delay) - try_num += 1 + @wraps(func) + def retry_func(self, *args, **kwargs): + for try_num in range(1, num_tries): + try: + return func(self, *args, **kwargs) + except AssertionError: + logger.warning(f"Assertion #{try_num} out of {num_tries} failed, retrying in {delay} seconds") + self.tearDown() + time.sleep(delay) + self.setUp() - return func(*args, **kwargs) + return func(self, *args, **kwargs) - return retry_func + return retry_func - return retry_decorator + +def _asset_folder_resources(folder): + resources = [] + options = {"max_results": 500} + try: + while True: + res = cloudinary.api.resources_by_asset_folder(folder, **options) + resources += res["resources"] + if not res.get("next_cursor"): + break + options["next_cursor"] = res["next_cursor"] + subfolders = cloudinary.api.subfolders(folder)["folders"] + except cloudinary.exceptions.NotFound: + return resources + + for subfolder in subfolders: + resources += _asset_folder_resources(subfolder["path"]) + + return resources + + +def _delete_assets(assets): + for resource_type in {a["resource_type"] for a in assets}: + public_ids = [a["public_id"] for a in assets if a["resource_type"] == resource_type] + for batch in range(0, len(public_ids), 100): + cloudinary.api.delete_resources(public_ids[batch:batch + 100], resource_type=resource_type) def delete_cld_folder_if_exists(folder, folder_mode = "fixed"): @@ -128,12 +154,14 @@ def delete_cld_folder_if_exists(folder, folder_mode = "fixed"): for resource_type in ("image", "raw", "video"): cloudinary.api.delete_resources_by_prefix(folder, resource_type=resource_type) else: - assets = query_cld_folder(folder, folder_mode) - for resource_type in {f["resource_type"] for f in assets.values()}: - cloudinary.api.delete_resources([f["public_id"] for f in assets.values() - if f["resource_type"] == resource_type], resource_type=resource_type) + _delete_assets(list(query_cld_folder(folder, folder_mode).values())) try: cloudinary.api.delete_folder(folder) except cloudinary.exceptions.NotFound: pass + except cloudinary.exceptions.BadRequest: + if folder_mode == "fixed": + raise + _delete_assets(_asset_folder_resources(folder)) + cloudinary.api.delete_folder(folder) diff --git a/test/test_modules/test_cli_sync.py b/test/test_modules/test_cli_sync.py index 177503b..287a70e 100644 --- a/test/test_modules/test_cli_sync.py +++ b/test/test_modules/test_cli_sync.py @@ -14,7 +14,7 @@ from test.helper_test import unique_suffix, RESOURCES_DIR, TEST_FILES_DIR, delete_cld_folder_if_exists, retry_assertion, \ get_request_url, get_params, URLLIB3_REQUEST from test.test_modules.test_cli_upload_dir import UPLOAD_MOCK_RESPONSE -from cloudinary_cli.utils.api_utils import get_folder_mode, _display_path, query_cld_folder +from cloudinary_cli.utils.api_utils import get_folder_mode, _display_path, query_cld_folder, cld_folder_exists from cloudinary_cli.modules.sync import SyncDir from cloudinary_cli.utils.utils import etag @@ -51,18 +51,15 @@ class TestCLISync(unittest.TestCase): DUPLICATE_NAME = unique_suffix("duplicate_name") - GRACE_PERIOD = 3 # seconds - folder_mode = "fixed" def setUp(self) -> None: self.folder_mode = get_folder_mode() delete_cld_folder_if_exists(self.CLD_SYNC_DIR, self.folder_mode) - time.sleep(1) + self._wait_for_cld_files(0) def tearDown(self) -> None: delete_cld_folder_if_exists(self.CLD_SYNC_DIR, self.folder_mode) - time.sleep(1) shutil.rmtree(self.LOCAL_SYNC_PULL_DIR, ignore_errors=True) @retry_assertion @@ -87,9 +84,6 @@ def test_cli_sync_push_non_existing_folder(self): def test_cli_sync_push_twice(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--push', '-F', TEST_FILES_DIR, self.CLD_SYNC_DIR]) self.assertEqual(0, result.exit_code) @@ -100,9 +94,6 @@ def test_cli_sync_push_twice(self): def test_cli_sync_push_out_of_sync(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--push', '-F', self.LOCAL_PARTIAL_SYNC_DIR, self.CLD_SYNC_DIR]) self.assertEqual(0, result.exit_code) @@ -117,9 +108,6 @@ def test_cli_sync_push_out_of_sync(self): def test_cli_sync_pull(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--pull', '-F', self.LOCAL_SYNC_PULL_DIR, self.CLD_SYNC_DIR]) self.assertEqual(0, result.exit_code, result.output) @@ -138,9 +126,6 @@ def test_cli_sync_pull_non_existing_folder(self): def test_cli_sync_pull_twice(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--pull', '-F', self.LOCAL_SYNC_PULL_DIR, self.CLD_SYNC_DIR]) self.assertEqual(0, result.exit_code) @@ -156,9 +141,6 @@ def test_cli_sync_pull_twice(self): def test_cli_sync_pull_out_of_sync(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - shutil.copytree(self.LOCAL_PARTIAL_SYNC_DIR, self.LOCAL_SYNC_PULL_DIR) result = self.runner.invoke(cli, ['sync', '--pull', '-F', self.LOCAL_SYNC_PULL_DIR, self.CLD_SYNC_DIR]) @@ -180,6 +162,7 @@ def _upload_sync_files(self, dir, optional_params=None): self.assertEqual(0, result.exit_code) self.assertIn("Synced | 12", result.output) self.assertIn("Done!", result.output) + self._wait_for_cld_files(12) @patch(URLLIB3_REQUEST) def test_sync_override_defaults(self, mocker): @@ -199,13 +182,10 @@ def test_sync_override_defaults(self, mocker): def test_cli_sync_duplicate_file_names_dynamic_folder_mode(self): self._upload_sync_files(TEST_FILES_DIR, ['-o', 'display_name', self.DUPLICATE_NAME]) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--pull', '-F', self.LOCAL_SYNC_PULL_DIR, self.CLD_SYNC_DIR]) self.assertEqual(0, result.exit_code) - self.assertIn("Found 0 items in local folder", result.output) + self.assertIn(f"Local folder '{self.LOCAL_SYNC_PULL_DIR}' does not exist.", result.output) self.assertIn("Downloading 12 files", result.output) for index in range(1, 6): self.assertIn(f"{self.DUPLICATE_NAME} ({index})", result.output) @@ -232,11 +212,15 @@ def _sync(self, direction, local_dir, *expected): self.assertIn(text, result.output) return result - def _wait_for_cld_files(self, count): - for _ in range(10): - if len(query_cld_folder(self.CLD_SYNC_DIR, self.folder_mode)) == count: - return - time.sleep(1) + def _wait_for_cld_files(self, count, timeout=15): + expected = (count, count > 0) + deadline = time.monotonic() + timeout + while (found := (len(query_cld_folder(self.CLD_SYNC_DIR, self.folder_mode)), + cld_folder_exists(self.CLD_SYNC_DIR))) != expected: + if time.monotonic() > deadline: + self.fail(f"Expected (items, folder exists) {expected} in Cloudinary folder " + f"'{self.CLD_SYNC_DIR}', found {found}") + time.sleep(0.5) def _assert_nothing_to_sync(self, direction, local_dir, count): result = self._sync(direction, local_dir, f"Skipping {count} items", "Done!") @@ -301,9 +285,6 @@ def test_cli_sync_push_include_hidden_skips_meta_file(self): def test_cli_sync_push_dry_run(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - result = self.runner.invoke(cli, ['sync', '--push', '-F', self.LOCAL_PARTIAL_SYNC_DIR, self.CLD_SYNC_DIR, '--dry-run']) # check that no files were uploaded @@ -316,9 +297,6 @@ def test_cli_sync_push_dry_run(self): def test_cli_sync_pull_dry_run(self): self._upload_sync_files(TEST_FILES_DIR) - # wait for indexing to be updated - time.sleep(self.GRACE_PERIOD) - shutil.copytree(self.LOCAL_PARTIAL_SYNC_DIR, self.LOCAL_SYNC_PULL_DIR) result = self.runner.invoke(cli, ['sync', '--pull', '-F', self.LOCAL_SYNC_PULL_DIR, self.CLD_SYNC_DIR, '--dry-run']) diff --git a/test/test_modules/test_cli_upload_dir.py b/test/test_modules/test_cli_upload_dir.py index 4812ade..287716b 100644 --- a/test/test_modules/test_cli_upload_dir.py +++ b/test/test_modules/test_cli_upload_dir.py @@ -1,4 +1,3 @@ -import time import unittest from unittest.mock import patch @@ -20,11 +19,9 @@ class TestCLIUploadDir(unittest.TestCase): def setUp(self) -> None: delete_cld_folder_if_exists(self.CLD_UPLOAD_DIR) - time.sleep(1) def tearDown(self) -> None: delete_cld_folder_if_exists(self.CLD_UPLOAD_DIR) - time.sleep(1) def test_cli_upload_dir(self): result = self.runner.invoke(cli, ["upload_dir", TEST_FILES_DIR] + self.FOLDER_OPTIONS)