fix: add_track(str) no longer reuses the source file's mkvmerge info

pymkv/MKVFile.py

@@ -657,11 +657,10 @@ def add_track(self, track: str | MKVTrack, new_file: bool = True) -> None:
         >>> mkv.add_track(track)  # doctest: +SKIP
         """
         if isinstance(track, str):
-            new_track = MKVTrack(
-                track,
-                mkvmerge_path=self.mkvmerge_path,
-                existing_info=self._info_json,
-            )
+            # `track` is an arbitrary path, so self._info_json (the mkvmerge output of the file
+            # this MKVFile was built from) does not describe it. Passing it here would make the
+            # new track report metadata of track 0 of the original file and skip verification.
+            new_track = MKVTrack(track, mkvmerge_path=self.mkvmerge_path)
             self._extracted_from_add_track(new_track, new_file)
         elif isinstance(track, MKVTrack):
             self._extracted_from_add_track(track, new_file)

pymkv/MKVTrack.py

@@ -66,7 +66,7 @@
 from pymkv.models import MkvMergeOutput
 from pymkv.TypeTrack import get_track_extension
 from pymkv.utils import prepare_mkvtoolnix_path
-from pymkv.Verifications import checking_file_path, get_file_info, verify_supported
+from pymkv.Verifications import checking_file_path, get_file_info, verify_mkvmerge
 
 if TYPE_CHECKING:
     from collections.abc import Iterable
@@ -189,6 +189,10 @@ def __init__(  # noqa: PLR0913
         self.mkvmerge_path = prepare_mkvtoolnix_path(mkvmerge_path)
         # Ensure _info_json is always MkvMergeOutput if possible, or None
         self._info_json: MkvMergeOutput | None = None
+        # Path that _info_json describes. None means "not probed yet", which also covers
+        # caller-supplied existing_info: it is trusted for the first file_path assignment
+        # and invalidated as soon as the path changes to something else.
+        self._info_path: str | None = None
         if existing_info:
             if isinstance(existing_info, dict):
                 # Convert legacy dict to Struct for strict usage
@@ -261,20 +265,40 @@ def file_path(self, file_path: str) -> None:
 
         This method checks if the provided file path is valid and supported by mkvmerge.
         If the file is valid, it sets the file path and resets the track_id to 0.
-        If existing_info is already set, skip the verify_supported check since the file
-        was already verified.
+
+        The mkvmerge output is probed once and cached, so the track_id setter does not
+        have to run mkvmerge a second time. Pointing the track at a different file drops
+        that cache, so the track always describes the file it currently points at.
+
+        When existing_info was supplied, it is trusted for the first assignment and no
+        probe happens at all. It stops being trusted as soon as the path changes.
 
         Args:
             file_path (str): The path to the file containing the track.
 
         Raises:
             ValueError: If the file is not a valid Matroska file or is not supported.
+            FileNotFoundError: If mkvmerge is not available at mkvmerge_path.
         """
         fp = checking_file_path(file_path)
-        if not self._info_json and not verify_supported(fp, 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 self._info_path is not None and fp != self._info_path:
+            # Pointing at a different file, whatever we cached describes the old one.
+            self._info_json = None
+        if self._info_json is None:
+            # verify_supported() would run mkvmerge -J and throw the result away, leaving
+            # the track_id setter to run it a second time. Probe once and keep the output.
+            # checking_file_path() above already validated the path, so only the mkvmerge
+            # binary still needs checking, and that lookup is cached.
+            if not verify_mkvmerge(self.mkvmerge_path):
+                msg = "mkvmerge is not at the specified path, add it there or change the mkvmerge_path property"
+                raise FileNotFoundError(msg)
+            info = get_file_info(fp, self.mkvmerge_path, check_path=False)
+            if not info.container.supported:
+                msg = f"The file '{file_path}' is not a valid Matroska file or is not supported."
+                raise ValueError(msg)
+            self._info_json = info
         self._file_path = fp
+        self._info_path = fp
         self.track_id = 0
 
     @property

tests/test_actions_track.py

@@ -218,7 +218,6 @@ def test_track_init_legacy_dict(dummy_mkv: Path) -> None:
 def test_track_repr(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
         rep = repr(t)
@@ -234,7 +233,6 @@ def test_track_pts_property(dummy_mkv: Path) -> None:
 
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
         # pts is 0 by default
@@ -246,7 +244,6 @@ def test_track_pts_property(dummy_mkv: Path) -> None:
 def test_track_tags_setter(dummy_mkv: Path, tmp_path: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
 
@@ -270,20 +267,16 @@ def test_track_tags_setter(dummy_mkv: Path, tmp_path: Path, single_video_info: M
 
 
 def test_track_file_path_setter_verification_failure(tmp_path: Path) -> None:
-    # Test that verify_supported failure raises ValueError
+    """An empty file is reported unsupported by mkvmerge, so the setter must reject it."""
     invalid = tmp_path / "invalid.mkv"
     invalid.touch()
-    with (
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=False),
-        pytest.raises(ValueError, match="not a valid Matroska file"),
-    ):
+    with pytest.raises(ValueError, match="not a valid Matroska file"):
         MKVTrack(str(invalid))
 
 
 def test_extract_silent_and_path(dummy_mkv: Path, tmp_path: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
     output = tmp_path / "extracted"
@@ -305,7 +298,6 @@ def test_extract_silent_and_path(dummy_mkv: Path, tmp_path: Path, single_video_i
 def test_file_id_setter_invalid_type(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
     with pytest.raises(ValueError, match="file_id must be an integer"):
@@ -315,7 +307,6 @@ def test_file_id_setter_invalid_type(dummy_mkv: Path, single_video_info: MkvMerg
 def test_track_id_setter_out_of_range(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
     with pytest.raises(IndexError, match="track index out of range"):
@@ -325,7 +316,6 @@ def test_track_id_setter_out_of_range(dummy_mkv: Path, single_video_info: MkvMer
 def test_language_setter_invalid(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
     with pytest.raises(
@@ -338,7 +328,6 @@ def test_language_setter_invalid(dummy_mkv: Path, single_video_info: MkvMergeOut
 def test_extract_track_name_fallback(dummy_mkv: Path, tmp_path: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
 
@@ -355,7 +344,6 @@ def test_extract_track_name_fallback(dummy_mkv: Path, tmp_path: Path, single_vid
 def test_extract_timestamps_silent(dummy_mkv: Path, tmp_path: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
 
@@ -370,11 +358,34 @@ def test_extract_timestamps_silent(dummy_mkv: Path, tmp_path: Path, single_video
 def test_extract_timestamps_default_path(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         t = MKVTrack(str(dummy_mkv))
 
     with patch("subprocess.run"):
         result = t.extract_timestamps(silent=True)
         assert str(dummy_mkv) in result
         assert "timestamps" in result
+
+
+def test_file_path_reassignment_refreshes_track_info(
+    get_path_test_file_two: Path,
+    get_path_test_srt: Path,
+) -> None:
+    """Pointing a track at another file must describe that file, not the previous one."""
+    track = MKVTrack(str(get_path_test_srt))
+    assert track.track_type == "subtitles"
+
+    track.file_path = str(get_path_test_file_two)
+
+    assert track.track_type == "video"
+    assert track.track_codec == MKVTrack(str(get_path_test_file_two)).track_codec
+
+
+def test_file_path_reassignment_verifies_new_file(get_path_test_srt: Path, tmp_path: Path) -> None:
+    """Verification must run again for the new path, not be skipped by the cached info."""
+    track = MKVTrack(str(get_path_test_srt))
+    invalid = tmp_path / "invalid.mkv"
+    invalid.touch()
+
+    with pytest.raises(ValueError, match="not a valid Matroska file"):
+        track.file_path = str(invalid)

tests/test_create_mkv.py

@@ -162,7 +162,6 @@ def test_order_tracks_by_file_id_keyerror(single_video_info: MkvMergeOutput) ->
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         mkv = MKVFile()
         t1 = MKVTrack("file1.mkv")
@@ -299,3 +298,34 @@ def test_add_track_with_string(get_path_test_file: Path) -> None:
     mkv = MKVFile()
     mkv.add_track(str(get_path_test_file))
     assert len(mkv.tracks) >= 1
+
+
+def test_add_track_with_string_reads_its_own_file(
+    get_path_test_file: Path,
+    get_path_test_srt: Path,
+) -> None:
+    """A track added by path must describe that path, not the file MKVFile was built from."""
+    mkv = MKVFile(str(get_path_test_file))
+    original_count = len(mkv.tracks)
+
+    mkv.add_track(str(get_path_test_srt))
+
+    assert len(mkv.tracks) == original_count + 1
+    added = mkv.tracks[-1]
+    assert added.file_path == str(get_path_test_srt)
+    assert added.track_type == "subtitles"
+    assert added.track_codec == MKVTrack(str(get_path_test_srt)).track_codec
+
+
+def test_add_track_with_string_rejects_unsupported_file(
+    get_path_test_file: Path,
+    get_base_path: Path,
+) -> None:
+    """Verification must still run for tracks added by path to a populated MKVFile."""
+    bogus = get_base_path / "not_a_media_file.txt"
+    bogus.write_text("definitely not media")
+
+    mkv = MKVFile(str(get_path_test_file))
+
+    with pytest.raises(ValueError, match="not a valid Matroska file or is not supported"):
+        mkv.add_track(str(bogus))

tests/test_language_loss_fix.py

@@ -7,7 +7,6 @@
 def test_language_loss_fix() -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
         mock_info.return_value.tracks = [MagicMock(id=0, codec="V_MS/VFW/FOURCC", type="video", start_pts=0)]
@@ -35,7 +34,6 @@ def test_language_loss_fix() -> None:
 def test_track_options_command_generation() -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
@@ -66,7 +64,6 @@ def test_track_options_skips_unnormalizable_language_ietf() -> None:
     # the same case, keeping the read- and write-side APIs consistent.
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
@@ -95,7 +92,6 @@ def test_track_options_emits_und_sentinel_language_ietf() -> None:
     # tag with ``und`` (rather than silently passing the source through).
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
@@ -123,7 +119,6 @@ def test_track_options_und_ietf_does_not_override_language() -> None:
     # and write APIs disagreeing about the same track.
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
@@ -151,7 +146,6 @@ def test_track_options_und_subtag_ietf_does_not_override_language() -> None:
     # ``language``. The write side must not emit the IETF tag in that case.
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):
@@ -182,7 +176,6 @@ def test_track_options_name_ietf_does_not_emit_and_reads_consistently() -> None:
     # ``--language`` at all.
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info") as mock_info,
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
         patch.object(sys.modules["pymkv.MKVFile"], "verify_mkvmerge", return_value=True),
         patch.object(sys.modules["pymkv.MKVTrack"], "checking_file_path", side_effect=lambda x: x),
     ):

tests/test_mkvtrack_language.py

@@ -49,7 +49,6 @@ def _make_track(dummy_mkv: Path, info: MkvMergeOutput) -> MKVTrack:
     """Build an MKVTrack against a dummy file path with mocked file probing."""
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         return MKVTrack(str(dummy_mkv))
 
@@ -137,7 +136,6 @@ def test_language_setter_empty_string_raises(dummy_mkv: Path, single_video_info:
 def test_language_setter_constructor_canonicalizes(dummy_mkv: Path, single_video_info: MkvMergeOutput) -> None:
     with (
         patch.object(sys.modules["pymkv.MKVTrack"], "get_file_info", return_value=single_video_info),
-        patch.object(sys.modules["pymkv.MKVTrack"], "verify_supported", return_value=True),
     ):
         track = MKVTrack(str(dummy_mkv), language="fra")
     assert track.language == "fre"