Skip to content

Commit

Permalink
feat: Unit tests for serialize_v1 for folders (#67)
Browse files Browse the repository at this point in the history
* Uni tests for serialize_v1 for folders

Signed-off-by: laurentsimon <[email protected]>

* typo + add sanity checks

Signed-off-by: laurentsimon <[email protected]>

* remove TODOs

Signed-off-by: laurentsimon <[email protected]>

* remove debug print

Signed-off-by: laurentsimon <[email protected]>

* use sets to simply sanity cekc assertion

Signed-off-by: laurentsimon <[email protected]>

* update

Signed-off-by: laurentsimon <[email protected]>

* rename file and folder names

Signed-off-by: laurentsimon <[email protected]>

* missing testdata

Signed-off-by: laurentsimon <[email protected]>

* add TODO in workflow

Signed-off-by: laurentsimon <[email protected]>

* Update model_signing/serialize_test.py

Co-authored-by: Mihai Maruseac <[email protected]>

* Update model_signing/serialize_test.py

Co-authored-by: Mihai Maruseac <[email protected]>

---------

Signed-off-by: laurentsimon <[email protected]>
Co-authored-by: Mihai Maruseac <[email protected]>
  • Loading branch information
laurentsimon and mihaimaruseac authored Nov 1, 2023
1 parent 0c0018c commit c362f3f
Show file tree
Hide file tree
Showing 3 changed files with 371 additions and 92 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,7 @@ jobs:
run: |
set -euo pipefail
source venv/bin/activate
pytest -v --full-trace .
# NOTE: option --full-trace may be useful for troubleshooting.
# TODO(#68): Remove the need to create this folder.
mkdir testdata
pytest -v .
12 changes: 11 additions & 1 deletion model_signing/serialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ def validate_signature_path(model_path: Path, sig_path: Path):
raise ValueError(f"{sig_path} must be in the folder root")


def is_relative_to(p: Path, path_list: [Path]) -> bool:
for e in path_list:
if p.is_relative_to(e):
return True
return False


# TODO(): add a context "AI model"?
class Serializer:
@staticmethod
Expand All @@ -137,7 +144,7 @@ def _ordered_files(path: Path, ignorepaths: [Path]) -> []:
filtered = []
total_size = 0
for child in children:
if child in ignorepaths:
if is_relative_to(child, ignorepaths):
continue

# To avoid bugs where we read the link rather than its target,
Expand Down Expand Up @@ -287,6 +294,9 @@ def _serialize_v1(path: Path, chunk: int, shard: int, signature_path: Path,
if not allow_symlinks and path.is_symlink():
raise ValueError(f"{str(path)} is a symlink")

if chunk < 0:
raise ValueError(f"{str(chunk)} is invalid")

if not path.is_file() and not path.is_dir():
raise ValueError(f"{str(path)} is not a dir or file")

Expand Down
Loading

0 comments on commit c362f3f

Please sign in to comment.