tests passing

Author: jayceslesar

pyiceberg/table/snapshots.py

@@ -25,7 +25,7 @@
 from pydantic import Field, PrivateAttr, model_serializer
 
 from pyiceberg.io import FileIO
-from pyiceberg.manifest import DataFile, DataFileContent, ManifestContent, ManifestFile, _manifests
+from pyiceberg.manifest import DataFile, DataFileContent, ManifestFile, _manifests
 from pyiceberg.partitioning import UNPARTITIONED_PARTITION_SPEC, PartitionSpec
 from pyiceberg.schema import Schema
 
@@ -251,18 +251,9 @@ def __str__(self) -> str:
         result_str = f"{operation}id={self.snapshot_id}{parent_id}{schema_id}"
         return result_str
 
-    def manifests(self, io: FileIO, content_filter: Optional[ManifestContent] = None) -> List[ManifestFile]:
-        """Return the manifests for the given snapshot.
-
-        Args:
-            io: The IO instance to read the manifest list.
-            content_filter: The content filter to apply to the manifests. One of ManifestContent.DATA or ManifestContent.DELETES.
-        """
-        return [
-            manifest
-            for manifest in _manifests(io, self.manifest_list)
-            if content_filter is None or manifest.content == content_filter
-        ]
+    def manifests(self, io: FileIO) -> List[ManifestFile]:
+        """Return the manifests for the given snapshot."""
+        return list(_manifests(io, self.manifest_list))
 
 
 class MetadataLogEntry(IcebergBaseModel):

pyiceberg/table/update/validate.py

@@ -59,8 +59,8 @@ def validation_history(
             manifests_files.extend(
                 [
                     manifest
-                    for manifest in snapshot.manifests(table.io, manifest_content_filter)
-                    if manifest.added_snapshot_id == snapshot.snapshot_id
+                    for manifest in snapshot.manifests(table.io)
+                    if manifest.added_snapshot_id == snapshot.snapshot_id and manifest.content == manifest_content_filter
                 ]
             )
 

tests/table/test_validate.py

@@ -16,22 +16,52 @@
 # under the License.
 # pylint:disable=redefined-outer-name,eval-used
 from typing import cast
+from unittest.mock import patch
 
-from pyiceberg.manifest import ManifestContent
+from pyiceberg.io import FileIO
+from pyiceberg.manifest import ManifestContent, ManifestFile
 from pyiceberg.table import Table
 from pyiceberg.table.snapshots import Operation, Snapshot
 from pyiceberg.table.update.validate import validation_history
 
 
 def test_validation_history(table_v2_with_extensive_snapshots: Table) -> None:
     """Test the validation history function."""
+    mock_manifests = {}
+
+    for i, snapshot in enumerate(table_v2_with_extensive_snapshots.snapshots()):
+        mock_manifest = ManifestFile(
+            manifest_path=f"foo/bar/{i}",
+            manifest_length=1,
+            partition_spec_id=1,
+            content=ManifestContent.DATA if i % 2 == 0 else ManifestContent.DELETES,
+            sequence_number=1,
+            min_sequence_number=1,
+            added_snapshot_id=snapshot.snapshot_id,
+        )
+
+        # Store the manifest for this specific snapshot
+        mock_manifests[snapshot.snapshot_id] = [mock_manifest]
+
+    expected_manifest_data_counts = len([m for m in mock_manifests.values() if m[0].content == ManifestContent.DATA]) - 1
+
     oldest_snapshot = table_v2_with_extensive_snapshots.snapshots()[0]
     newest_snapshot = cast(Snapshot, table_v2_with_extensive_snapshots.current_snapshot())
-    manifests, snapshots = validation_history(
-        table_v2_with_extensive_snapshots,
-        newest_snapshot,
-        oldest_snapshot,
-        {Operation.APPEND},
-        ManifestContent.DATA,
-    )
-    assert len(snapshots) == 2
+
+    def mock_read_manifest_side_effect(self: Snapshot, io: FileIO) -> list[ManifestFile]:
+        """Mock the manifests method to use the snapshot_id for lookup."""
+        snapshot_id = self.snapshot_id
+        if snapshot_id in mock_manifests:
+            return mock_manifests[snapshot_id]
+        return []
+
+    with patch("pyiceberg.table.snapshots.Snapshot.manifests", new=mock_read_manifest_side_effect):
+        manifests, snapshots = validation_history(
+            table_v2_with_extensive_snapshots,
+            newest_snapshot,
+            oldest_snapshot,
+            {Operation.APPEND},
+            ManifestContent.DATA,
+        )
+
+        assert len(manifests) == expected_manifest_data_counts