Skip to content

Commit 2307c4c

Browse files
oleksandr-ncReview
andauthored
fix(tests): make zip-download assertions order-tolerant (#429)
Nextcloud server PR nextcloud/server#60225 dropped the implicit `ORDER BY name ASC` from `Cache::getFolderContentsById`, so the order of children inside a downloaded zip is now whatever the DB returns and varies by backend. On PostgreSQL this breaks test_download_as_zip / test_download_as_zip_async, which asserted on positional indices. Rebuild `_test_download_as_zip` around a `{filename: size}` dict and check membership + sizes instead of positions. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved robustness of ZIP download tests to handle varying file ordering, ensuring reliable validation of downloaded archive contents. <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cloud-py-api/nc_py_api/pull/429?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Review <review@local>
1 parent 01ce7b0 commit 2307c4c

1 file changed

Lines changed: 27 additions & 20 deletions

File tree

tests/actual_tests/files_test.py

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -876,29 +876,36 @@ def test_fs_node_str(nc_any):
876876

877877

878878
def _test_download_as_zip(result: Path, n: int):
879+
# Server-side child ordering inside the archive is not guaranteed (depends on
880+
# the database backend after nextcloud/server#60225), so assert on membership
881+
# and sizes rather than positions.
882+
with zipfile.ZipFile(result, "r") as zip_ref:
883+
actual = {info.filename: info.file_size for info in zip_ref.filelist}
879884
if n == 1:
880-
with zipfile.ZipFile(result, "r") as zip_ref:
881-
assert zip_ref.filelist[0].filename == "test_dir/"
882-
assert not zip_ref.filelist[0].file_size
883-
assert zip_ref.filelist[1].filename == "test_dir/subdir/"
884-
assert not zip_ref.filelist[1].file_size
885-
assert zip_ref.filelist[2].filename == "test_dir/subdir/test_12345_text.txt"
886-
assert zip_ref.filelist[2].file_size == 5
887-
assert zip_ref.filelist[3].filename == "test_dir/subdir/test_64_bytes.bin"
888-
assert zip_ref.filelist[3].file_size == 64
889-
assert len(zip_ref.filelist) == 11
885+
fixed = {
886+
"test_dir/": 0,
887+
"test_dir/subdir/": 0,
888+
"test_dir/subdir/test_empty_text.txt": 0,
889+
"test_dir/subdir/test_64_bytes.bin": 64,
890+
"test_dir/subdir/test_12345_text.txt": 5,
891+
"test_dir/test_empty_child_dir/": 0,
892+
"test_dir/test_empty_text.txt": 0,
893+
"test_dir/test_64_bytes.bin": 64,
894+
"test_dir/test_12345_text.txt": 5,
895+
}
896+
variable = {"test_dir/subdir/test_generated_image.png", "test_dir/test_generated_image.png"}
897+
for name, size in fixed.items():
898+
assert actual.get(name) == size, f"{name}: expected size {size}, got {actual.get(name)!r}"
899+
for name in variable:
900+
assert actual.get(name, 0) > 0, f"{name}: expected non-empty file"
901+
assert actual.keys() == fixed.keys() | variable
890902
elif n == 2:
891-
with zipfile.ZipFile(result, "r") as zip_ref:
892-
assert zip_ref.filelist[0].filename == "test_empty_dir_in_dir/"
893-
assert not zip_ref.filelist[0].file_size
894-
assert zip_ref.filelist[1].filename == "test_empty_dir_in_dir/test_empty_child_dir/"
895-
assert not zip_ref.filelist[1].file_size
896-
assert len(zip_ref.filelist) == 2
903+
assert actual == {
904+
"test_empty_dir_in_dir/": 0,
905+
"test_empty_dir_in_dir/test_empty_child_dir/": 0,
906+
}
897907
else:
898-
with zipfile.ZipFile(result, "r") as zip_ref:
899-
assert zip_ref.filelist[0].filename == "test_empty_dir/"
900-
assert not zip_ref.filelist[0].file_size
901-
assert len(zip_ref.filelist) == 1
908+
assert actual == {"test_empty_dir/": 0}
902909

903910

904911
def test_download_as_zip(nc):

0 commit comments

Comments
 (0)