Skip to content

Core: Use mocks in TrackedFile tests #17133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdblue merged 4 commits into from
Jul 13, 2026
+28 −67
Merged

Core: Use mocks in TrackedFile tests #17133

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
83d6309
Core: Use mocks in TrackedFile tests.
rdblue Jul 7, 2026
4488371
Use isSameAs to test injected copies.
rdblue Jul 8, 2026
9f0dd2d
Core: Also mock DeletionVector and ManifestInfo.
rdblue Jul 13, 2026
8faa8b6
Apply spotless.
rdblue Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 16 additions & 31 deletions core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@
import static org.assertj.core.api.Assertions.assertThat;

import java.nio.ByteBuffer;
import java.util.Comparator;
import java.util.List;
import org.apache.iceberg.TestHelpers.RoundTripSerializer;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.types.Comparators;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mockito;

class TestTrackedFileStruct {
private static final int FORMAT_VERSION_V4 = 4;
Expand All @@ -41,15 +40,16 @@ class TestTrackedFileStruct {
private static final List<Types.NestedField> FIELDS =
TrackedFile.schemaWithContentStats(PARTITION_TYPE, Types.StructType.of()).fields();

private static final Comparator<StructLike> PARTITION_COMPARATOR =
Comparators.forType(PARTITION_TYPE);
private static final Comparator<StructLike> TRACKING_COMPARATOR =
Comparators.forType(Tracking.schema());
private static final Tracking TRACKING = Mockito.mock(Tracking.class);
private static final Tracking TRACKING_COPY = Mockito.mock(Tracking.class);

private static final Tracking TRACKING =
new TrackingStruct(EntryStatus.ADDED, 42L, 10L, 10L, 43L, 1000L, null, null);
private static final PartitionData PARTITION = Mockito.mock(PartitionData.class);
private static final PartitionData PARTITION_COPY = Mockito.mock(PartitionData.class);

private static final PartitionData PARTITION = newPartition(7, "music");
static {
Mockito.when(TRACKING.copy()).thenReturn(TRACKING_COPY);
Mockito.when(PARTITION.copy()).thenReturn(PARTITION_COPY);
}

private static final DeletionVectorStruct DELETION_VECTOR =
DeletionVectorStruct.builder()
Expand All @@ -72,15 +72,7 @@ class TestTrackedFileStruct {
.minSequenceNumber(5L)
.build();

private static final ContentStats CONTENT_STATS =
BaseContentStats.builder()
.withTableSchema(
new Schema(
Types.NestedField.optional(1, "id", Types.IntegerType.get()),
Types.NestedField.optional(2, "data", Types.FloatType.get())))
.withFieldStats(BaseFieldStats.builder().fieldId(1).build())
.withFieldStats(BaseFieldStats.builder().fieldId(2).build())
.build();
private static final ContentStats CONTENT_STATS = Mockito.mock(ContentStats.class);

@rdblue rdblue Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this conflicts with #17159 (https://github.com/apache/iceberg/pull/17159/changes#diff-2e1fd2a7ed0bab2997b0ccc363eb220bc17cbe6e8db8ba6a199826091d8b4b09L75-R82). I'll rebase and fix this depending on which one makes it in first.


@Test
void fieldAccess() {
Expand Down Expand Up @@ -224,8 +216,7 @@ void copy() {
TrackedFile copy = file.copy();

assertThat(copy).isInstanceOf(TrackedFileStruct.class);
assertThat(TRACKING_COMPARATOR.compare((StructLike) copy.tracking(), (StructLike) TRACKING))
.isEqualTo(0);
assertThat(copy.tracking()).isSameAs(TRACKING_COPY);
assertThat(copy.contentType()).isEqualTo(FileContent.DATA);
assertThat(copy.formatVersion()).isEqualTo(FORMAT_VERSION_V4);
assertThat(copy.location()).isEqualTo("s3://bucket/data/00000-0-file.parquet");
Expand All @@ -240,12 +231,9 @@ void copy() {
assertThat(copy.keyMetadata()).isEqualTo(ByteBuffer.wrap(new byte[] {1, 2, 3}));
assertThat(copy.splitOffsets()).containsExactly(100L, 200L);
assertThat(copy.equalityIds()).containsExactly(1, 2, 3);

assertThat(PARTITION_COMPARATOR.compare(copy.partition(), PARTITION)).isEqualTo(0);
assertThat(copy.partition()).isSameAs(PARTITION_COPY);
Comment thread
rdblue marked this conversation as resolved.

// mutable fields are deep-copied, not shared with the original
assertThat(copy.tracking()).isNotSameAs(file.tracking());

@stevenzwu stevenzwu Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it might make more sense to keep the two isSameAs assertions here to check they are deep copied and not shared with the original object, as the comment above described.

@rdblue rdblue Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My rationale is that isSameAs(x) implies isNotSameAs(y) if x is not the same as y because the relation is transitive. By verifying isSameAs, this is already ensuring it is the result of copy.

We could go further and use this as a validation that copy produces a different object (it does not always) but that is the responsibility of the tests for TrackingStruct, not TrackedFileStruct.

assertThat(copy.partition()).isNotSameAs(file.partition());
assertThat(copy.deletionVector()).isNotSameAs(file.deletionVector());
Comment thread
rdblue marked this conversation as resolved.
Outdated
assertThat(copy.manifestInfo()).isNotSameAs(file.manifestInfo());
assertThat(copy.keyMetadata()).isNotSameAs(file.keyMetadata());
Expand Down Expand Up @@ -280,12 +268,12 @@ void structLikeSize() {
void serializationRoundTrip(RoundTripSerializer<TrackedFileStruct> serializer) throws Exception {
TrackedFileStruct file =
new TrackedFileStruct(
TRACKING,
null, // TrackingStruct has its own serialization tests
FileContent.DATA,
FORMAT_VERSION_V4,
"s3://bucket/data/file.parquet",
FileFormat.PARQUET,
PARTITION,
null, // PartitionData has its own serialization tests
100L,
1024L,
7,
Expand All @@ -299,15 +287,12 @@ void serializationRoundTrip(RoundTripSerializer<TrackedFileStruct> serializer) t

TrackedFileStruct deserialized = serializer.apply(file);

assertThat(
TRACKING_COMPARATOR.compare(
(StructLike) deserialized.tracking(), (StructLike) TRACKING))
.isEqualTo(0);
assertThat(deserialized.tracking()).isNull();
assertThat(deserialized.contentType()).isEqualTo(FileContent.DATA);
assertThat(deserialized.formatVersion()).isEqualTo(FORMAT_VERSION_V4);
assertThat(deserialized.location()).isEqualTo("s3://bucket/data/file.parquet");
assertThat(deserialized.fileFormat()).isEqualTo(FileFormat.PARQUET);
assertThat(PARTITION_COMPARATOR.compare(deserialized.partition(), PARTITION)).isEqualTo(0);
assertThat(deserialized.partition()).isNull();
assertThat(deserialized.recordCount()).isEqualTo(100L);
assertThat(deserialized.fileSizeInBytes()).isEqualTo(1024L);
assertThat(deserialized.specId()).isEqualTo(7);
Expand Down
Loading