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

Open
rdblue wants to merge 2 commits into
base: main
Choose a base branch
from
+16 −31
Open

Core: Use mocks in TrackedFile tests #17133

Changes from all commits
Commits
Show all changes
2 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
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);

@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.

assertThat(copy.partition()).isNotSameAs(file.partition());
assertThat(copy.deletionVector()).isNotSameAs(file.deletionVector());

@gaborkaszab gaborkaszab Jul 8, 2026

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.

Would it make sense to introduce mocks for the below nested fields too? Tracking and partition doesn't seem any special from this aspect.

@rdblue rdblue Jul 8, 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.

The issue with this is that we do not currently have copy methods for them. I think that we should follow up to do this.

@gaborkaszab gaborkaszab Jul 9, 2026

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.

DeletionVector and ManifestInfo both have their copy() function. See here and here.

keyMetadata is a byte array not a class, we won't be able to do so for this, but for the other 2 members we could.

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