From f6b8ca2af3a266120bbc8d26166eba7fc3b5068c Mon Sep 17 00:00:00 2001 From: GitBib <15717621+GitBib@users.noreply.github.com> Date: Mon, 3 Jun 2024 01:03:32 +0300 Subject: [PATCH 1/2] Updated type verification and enhanced file validation Expanded the type check for 'mkvmerge_path' to now include list, os.PathLike, and None aside from string. Improved the interface of the 'file_path' setter of the MKVTrack to better handle unsupported files. Added a verification step when initializing an MKVFile to ensure that the file is a supported Matroska file. Updated the 'MKVFile' class and unit tests to reflect these changes. --- pymkv/MKVFile.py | 8 ++++++-- pymkv/MKVTrack.py | 6 +++--- pymkv/Verifications.py | 4 ++-- tests/test_open_files.py | 8 ++++++++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pymkv/MKVFile.py b/pymkv/MKVFile.py index 3892bcc..e45e514 100644 --- a/pymkv/MKVFile.py +++ b/pymkv/MKVFile.py @@ -50,7 +50,7 @@ from pymkv.MKVTrack import MKVTrack from pymkv.Timestamp import Timestamp from pymkv.utils import prepare_mkvtoolnix_path -from pymkv.Verifications import checking_file_path, verify_mkvmerge +from pymkv.Verifications import checking_file_path, verify_mkvmerge, verify_supported class MKVFile: @@ -104,10 +104,14 @@ def __init__( # noqa: C901, PLR0912 self.attachments = [] self._number_file = 0 - if not verify_mkvmerge(mkvmerge_path=mkvmerge_path): + if not verify_mkvmerge(mkvmerge_path=self.mkvmerge_path): msg = "mkvmerge is not at the specified path, add it there or changed mkvmerge_path property" raise FileNotFoundError(msg) + if not verify_supported(file_path, mkvmerge_path=self.mkvmerge_path): + msg = f"The file '{file_path}' is not a valid Matroska file or is not supported." + raise ValueError(msg) + if file_path is not None: # add file title file_path = checking_file_path(file_path) diff --git a/pymkv/MKVTrack.py b/pymkv/MKVTrack.py index c9bf322..1381f99 100644 --- a/pymkv/MKVTrack.py +++ b/pymkv/MKVTrack.py @@ -49,7 +49,7 @@ from pymkv.ISO639_2 import is_iso639_2 from pymkv.TypeTrack import get_track_extension from pymkv.utils import prepare_mkvtoolnix_path -from pymkv.Verifications import verify_supported +from pymkv.Verifications import checking_file_path, verify_supported class MKVTrack: @@ -189,9 +189,9 @@ def file_path(self) -> str: @file_path.setter def file_path(self, file_path: str) -> None: - file_path = str(Path(file_path).expanduser()) + file_path = checking_file_path(file_path) if not verify_supported(file_path, mkvmerge_path=self.mkvmerge_path): - msg = '"{}" is not a supported file' + msg = f"The file '{file_path}' is not a valid Matroska file or is not supported." raise ValueError(msg) self._file_path = file_path self.track_id = 0 diff --git a/pymkv/Verifications.py b/pymkv/Verifications.py index 6d4a57b..37224ec 100644 --- a/pymkv/Verifications.py +++ b/pymkv/Verifications.py @@ -58,7 +58,7 @@ def verify_mkvmerge(mkvmerge_path: str | list | os.PathLike | None = "mkvmerge") return bool(match("mkvmerge.*", output)) -def verify_matroska(file_path: str | os.PathLike, mkvmerge_path: str = "mkvmerge") -> bool: +def verify_matroska(file_path: str | os.PathLike, mkvmerge_path: str | list | os.PathLike | None = "mkvmerge") -> bool: """ Parameters ---------- @@ -93,7 +93,7 @@ def verify_matroska(file_path: str | os.PathLike, mkvmerge_path: str = "mkvmerge msg = "mkvmerge is not at the specified path, add it there or change the mkvmerge_path property" raise FileNotFoundError(msg) try: - info_json = json.loads( + info_json: dict = json.loads( sp.check_output( [*prepare_mkvtoolnix_path(mkvmerge_path), "-J", checking_file_path(file_path)], # noqa: S603 ).decode(), diff --git a/tests/test_open_files.py b/tests/test_open_files.py index dc3a92e..a726237 100644 --- a/tests/test_open_files.py +++ b/tests/test_open_files.py @@ -22,3 +22,11 @@ def test_mux_file(get_base_path: Path, get_path_test_file: Path) -> None: def test_file_not_found() -> None: with pytest.raises(FileNotFoundError): MKVFile("file-zero.mkv") + + +def test_file_not_support() -> None: + with pytest.raises( + ValueError, + match="The file 'tests/conftest.py' is not a valid Matroska file or is not supported.", + ): + MKVFile("tests/conftest.py") From 0c6c5e87d8f28f4b572fed12094f750655d5abd6 Mon Sep 17 00:00:00 2001 From: GitBib <15717621+GitBib@users.noreply.github.com> Date: Mon, 3 Jun 2024 01:18:16 +0300 Subject: [PATCH 2/2] Add tests for MKVFile and MKVTrack classes A new test file, 'test_verify.py', has been added to test the 'verify_matroska' function of the MKVFile and MKVTrack classes. The unit tests assert whether the function results are correct when provided with valid and invalid input parameters. The 'test_open_files.py' file has also been updated to include additional test scenarios for these classes. --- tests/test_open_files.py | 10 +++++++++- tests/test_verify.py | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/test_verify.py diff --git a/tests/test_open_files.py b/tests/test_open_files.py index a726237..26d52b0 100644 --- a/tests/test_open_files.py +++ b/tests/test_open_files.py @@ -2,7 +2,7 @@ import pytest -from pymkv import MKVFile +from pymkv import MKVFile, MKVTrack def test_open_file(get_path_test_file: Path) -> None: @@ -30,3 +30,11 @@ def test_file_not_support() -> None: match="The file 'tests/conftest.py' is not a valid Matroska file or is not supported.", ): MKVFile("tests/conftest.py") + + +def test_track_not_support() -> None: + with pytest.raises( + ValueError, + match="The file 'tests/conftest.py' is not a valid Matroska file or is not supported.", + ): + MKVTrack("tests/conftest.py") diff --git a/tests/test_verify.py b/tests/test_verify.py new file mode 100644 index 0000000..be62507 --- /dev/null +++ b/tests/test_verify.py @@ -0,0 +1,19 @@ +from pathlib import Path + +import pytest + +from pymkv import verify_matroska + + +def test_verify_matroska_true(get_path_test_file: Path) -> None: + assert verify_matroska(get_path_test_file) is True + + +def test_verify_matroska_error(get_path_test_file: Path) -> None: + with pytest.raises(FileNotFoundError): + verify_matroska(get_path_test_file, "mkvmerge-test") + + +def test_verify_matroska_false(get_path_test_file: Path) -> None: + with pytest.raises(KeyError): + verify_matroska("tests/conftest.py")