Skip to content

GH-3615: Use assertThat checks in parquet-variant tests #3617

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
nastra wants to merge 2 commits into
base: master
Choose a base branch
from
Open

GH-3615: Use assertThat checks in parquet-variant tests #3617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5c5779
GH-3615: Introduce AssertJ for testing
nastra Jun 19, 2026
7215ca4
GH-3615: Use assertThat checks in parquet-variant tests
nastra Jun 19, 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
49 changes: 23 additions & 26 deletions parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
*/
package org.apache.parquet.variant;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.time.LocalDate;
import org.junit.Assert;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -77,7 +79,7 @@ public void testEmptyArray() {
Variant value = new Variant(ByteBuffer.wrap(new byte[] {0b0011, 0x00}), VariantTestUtil.EMPTY_METADATA);
VariantTestUtil.testVariant(value, v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(0, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(0);
});
}

Expand All @@ -87,7 +89,7 @@ public void testEmptyLargeArray() {
ByteBuffer.wrap(new byte[] {0b10011, 0x00, 0x00, 0x00, 0x00}), VariantTestUtil.EMPTY_METADATA);
VariantTestUtil.testVariant(value, v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(0, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(0);
});
}

Expand All @@ -98,7 +100,7 @@ public void testLargeArraySize() {
VariantTestUtil.EMPTY_METADATA);
VariantTestUtil.testVariant(value, v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(511, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(511);
});
}

Expand All @@ -111,26 +113,24 @@ public void testMixedArray() {

VariantTestUtil.testVariant(value, v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(5, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(5);
VariantTestUtil.checkType(v.getElementAtIndex(0), VariantUtil.PRIMITIVE, Variant.Type.DATE);
Assert.assertEquals(
LocalDate.parse("2025-04-17"),
LocalDate.ofEpochDay(v.getElementAtIndex(0).getInt()));
assertThat(LocalDate.ofEpochDay(v.getElementAtIndex(0).getInt())).isEqualTo(LocalDate.parse("2025-04-17"));
VariantTestUtil.checkType(v.getElementAtIndex(1), VariantUtil.PRIMITIVE, Variant.Type.BOOLEAN);
Assert.assertTrue(v.getElementAtIndex(1).getBoolean());
assertThat(v.getElementAtIndex(1).getBoolean()).isTrue();
VariantTestUtil.checkType(v.getElementAtIndex(2), VariantUtil.PRIMITIVE, Variant.Type.INT);
Assert.assertEquals(1234567890, v.getElementAtIndex(2).getInt());
assertThat(v.getElementAtIndex(2).getInt()).isEqualTo(1234567890);
VariantTestUtil.checkType(v.getElementAtIndex(3), VariantUtil.PRIMITIVE, Variant.Type.STRING);
Assert.assertEquals("variant", v.getElementAtIndex(3).getString());
assertThat(v.getElementAtIndex(3).getString()).isEqualTo("variant");
VariantTestUtil.checkType(v.getElementAtIndex(4), VariantUtil.ARRAY, Variant.Type.ARRAY);

Variant nestedV = v.getElementAtIndex(4);
Assert.assertEquals(3, nestedV.numArrayElements());
assertThat(nestedV.numArrayElements()).isEqualTo(3);
VariantTestUtil.checkType(nestedV.getElementAtIndex(0), VariantUtil.PRIMITIVE, Variant.Type.INT);
Assert.assertEquals(1234567890, nestedV.getElementAtIndex(0).getInt());
assertThat(nestedV.getElementAtIndex(0).getInt()).isEqualTo(1234567890);
VariantTestUtil.checkType(nestedV.getElementAtIndex(1), VariantUtil.PRIMITIVE, Variant.Type.NULL);
VariantTestUtil.checkType(nestedV.getElementAtIndex(2), VariantUtil.SHORT_STR, Variant.Type.STRING);
Assert.assertEquals("c", nestedV.getElementAtIndex(2).getString());
assertThat(nestedV.getElementAtIndex(2).getString()).isEqualTo("c");
});
}

Expand All @@ -141,13 +141,13 @@ private void testArrayOffsetSize(String randomString) {

VariantTestUtil.testVariant(value, v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(3, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(3);
VariantTestUtil.checkType(v.getElementAtIndex(0), VariantUtil.PRIMITIVE, Variant.Type.STRING);
Assert.assertEquals(randomString, v.getElementAtIndex(0).getString());
assertThat(v.getElementAtIndex(0).getString()).isEqualTo(randomString);
VariantTestUtil.checkType(v.getElementAtIndex(1), VariantUtil.PRIMITIVE, Variant.Type.BOOLEAN);
Assert.assertTrue(v.getElementAtIndex(1).getBoolean());
assertThat(v.getElementAtIndex(1).getBoolean()).isTrue();
VariantTestUtil.checkType(v.getElementAtIndex(2), VariantUtil.PRIMITIVE, Variant.Type.INT);
Assert.assertEquals(1234567890, v.getElementAtIndex(2).getInt());
assertThat(v.getElementAtIndex(2).getInt()).isEqualTo(1234567890);
});
}

Expand All @@ -171,13 +171,10 @@ public void testArrayFourByteOffset() {

@Test
public void testInvalidArray() {
try {
// An object header
Variant value = new Variant(ByteBuffer.wrap(new byte[] {0b1000010}), VariantTestUtil.EMPTY_METADATA);
value.numArrayElements();
Assert.fail("Expected exception not thrown");
} catch (Exception e) {
Assert.assertEquals("Cannot read OBJECT value as ARRAY", e.getMessage());
}
// An object header
Variant value = new Variant(ByteBuffer.wrap(new byte[] {0b1000010}), VariantTestUtil.EMPTY_METADATA);
assertThatThrownBy(value::numArrayElements)

@nastra nastra Jun 19, 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.

Previously with Assert.fail we would have a larger block where it wasn't clear code line of code would actually throw the exception and what error msg would be returned. We're more explicit now in terms of testing error conditions, because we check the underlying exception and the error msg.

.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot read OBJECT value as ARRAY");
}
}
123 changes: 49 additions & 74 deletions parquet-variant/src/test/java/org/apache/parquet/variant/TestVariantArrayBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
*/
package org.apache.parquet.variant;

import org.junit.Assert;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -33,7 +35,7 @@ public void testEmptyArrayBuilder() {
b.endArray();
VariantTestUtil.testVariant(b.build(), v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(0, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(0);
});
}

Expand All @@ -47,10 +49,10 @@ public void testLargeArraySizeBuilder() {
b.endArray();
VariantTestUtil.testVariant(b.build(), v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(511, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(511);
for (int i = 0; i < 511; i++) {
VariantTestUtil.checkType(v.getElementAtIndex(i), VariantUtil.PRIMITIVE, Variant.Type.INT);
Assert.assertEquals(i, v.getElementAtIndex(i).getInt());
assertThat(v.getElementAtIndex(i).getInt()).isEqualTo(i);
}
});
}
Expand Down Expand Up @@ -82,28 +84,28 @@ public void testMixedArrayBuilder() {

VariantTestUtil.testVariant(b.build(), v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(4, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(4);
VariantTestUtil.checkType(v.getElementAtIndex(0), VariantUtil.PRIMITIVE, Variant.Type.BOOLEAN);
Assert.assertTrue(v.getElementAtIndex(0).getBoolean());
assertThat(v.getElementAtIndex(0).getBoolean()).isTrue();

VariantTestUtil.checkType(v.getElementAtIndex(1), VariantUtil.OBJECT, Variant.Type.OBJECT);
Assert.assertEquals(1, v.getElementAtIndex(1).numObjectElements());
assertThat(v.getElementAtIndex(1).numObjectElements()).isEqualTo(1);
VariantTestUtil.checkType(
v.getElementAtIndex(1).getFieldByKey("key"), VariantUtil.PRIMITIVE, Variant.Type.INT);
Assert.assertEquals(321, v.getElementAtIndex(1).getFieldByKey("key").getInt());
assertThat(v.getElementAtIndex(1).getFieldByKey("key").getInt()).isEqualTo(321);

VariantTestUtil.checkType(v.getElementAtIndex(2), VariantUtil.PRIMITIVE, Variant.Type.LONG);
Assert.assertEquals(1234567890, v.getElementAtIndex(2).getLong());
assertThat(v.getElementAtIndex(2).getLong()).isEqualTo(1234567890);

VariantTestUtil.checkType(v.getElementAtIndex(3), VariantUtil.ARRAY, Variant.Type.ARRAY);
Variant nested = v.getElementAtIndex(3);
Assert.assertEquals(3, nested.numArrayElements());
assertThat(nested.numArrayElements()).isEqualTo(3);
VariantTestUtil.checkType(nested.getElementAtIndex(0), VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(0, nested.getElementAtIndex(0).numArrayElements());
assertThat(nested.getElementAtIndex(0).numArrayElements()).isEqualTo(0);
VariantTestUtil.checkType(nested.getElementAtIndex(1), VariantUtil.SHORT_STR, Variant.Type.STRING);
Assert.assertEquals("variant", nested.getElementAtIndex(1).getString());
assertThat(nested.getElementAtIndex(1).getString()).isEqualTo("variant");
VariantTestUtil.checkType(nested.getElementAtIndex(2), VariantUtil.OBJECT, Variant.Type.OBJECT);
Assert.assertEquals(0, nested.getElementAtIndex(2).numObjectElements());
assertThat(nested.getElementAtIndex(2).numObjectElements()).isEqualTo(0);
});
}

Expand All @@ -126,11 +128,11 @@ public void testNestedBuilder() {
for (int i = 1000; i >= 0; i--) {
VariantTestUtil.checkType(curr, VariantUtil.ARRAY, Variant.Type.ARRAY);
if (i == 0) {
Assert.assertEquals(0, curr.numArrayElements());
assertThat(curr.numArrayElements()).isEqualTo(0);
} else {
Assert.assertEquals(2, curr.numArrayElements());
assertThat(curr.numArrayElements()).isEqualTo(2);
VariantTestUtil.checkType(curr.getElementAtIndex(0), VariantUtil.SHORT_STR, Variant.Type.STRING);
Assert.assertEquals("str" + i, curr.getElementAtIndex(0).getString());
assertThat(curr.getElementAtIndex(0).getString()).isEqualTo("str" + i);
curr = curr.getElementAtIndex(1);
}
}
Expand All @@ -147,13 +149,13 @@ private void testArrayOffsetSizeBuilder(String randomString) {

VariantTestUtil.testVariant(b.build(), v -> {
VariantTestUtil.checkType(v, VariantUtil.ARRAY, Variant.Type.ARRAY);
Assert.assertEquals(3, v.numArrayElements());
assertThat(v.numArrayElements()).isEqualTo(3);
VariantTestUtil.checkType(v.getElementAtIndex(0), VariantUtil.PRIMITIVE, Variant.Type.STRING);
Assert.assertEquals(randomString, v.getElementAtIndex(0).getString());
assertThat(v.getElementAtIndex(0).getString()).isEqualTo(randomString);
VariantTestUtil.checkType(v.getElementAtIndex(1), VariantUtil.PRIMITIVE, Variant.Type.BOOLEAN);
Assert.assertTrue(v.getElementAtIndex(1).getBoolean());
assertThat(v.getElementAtIndex(1).getBoolean()).isTrue();
VariantTestUtil.checkType(v.getElementAtIndex(2), VariantUtil.PRIMITIVE, Variant.Type.LONG);
Assert.assertEquals(1234567890, v.getElementAtIndex(2).getLong());
assertThat(v.getElementAtIndex(2).getLong()).isEqualTo(1234567890);
});
}

Expand All @@ -179,60 +181,45 @@ public void testArrayFourByteOffsetBuilder() {
public void testMissingEndArray() {
VariantBuilder b = new VariantBuilder();
b.startArray();
try {
b.build();
Assert.fail("Expected Exception when calling build() without endArray()");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::build)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call build() while an array is being built. Must call endArray() first.");
}

@Test
public void testMissingStartArray() {
VariantBuilder b = new VariantBuilder();
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() without startArray()");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() without calling startArray() first.");
}

@Test
public void testInvalidAppendDuringArray() {
VariantBuilder b = new VariantBuilder();
b.startArray();
try {
b.appendInt(1);
Assert.fail("Expected Exception when calling append() before endArray()");
} catch (Exception e) {
// expected
}
assertThatThrownBy(() -> b.appendInt(1))
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call append() methods while an array is being built. Must call endArray() first.");
}

@Test
public void testStartArrayEndObject() {
VariantBuilder b = new VariantBuilder();
VariantArrayBuilder obj = b.startArray();
try {
obj.endObject();
Assert.fail("Expected Exception when calling endObject() while building array");
} catch (Exception e) {
// expected
}
assertThatThrownBy(obj::endObject)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endObject() without calling startObject() first.");
}

@Test
public void testOpenNestedObject() {
VariantBuilder b = new VariantBuilder();
VariantArrayBuilder arr = b.startArray();
arr.startObject();
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() with an open nested object");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() while a nested object/array is still open.");
}

@Test
Expand All @@ -241,12 +228,9 @@ public void testOpenNestedObjectWithKey() {
VariantArrayBuilder arr = b.startArray();
VariantObjectBuilder nested = arr.startObject();
nested.appendKey("nested");
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() with an open nested object");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() while a nested object/array is still open.");
}

@Test
Expand All @@ -256,25 +240,19 @@ public void testOpenNestedObjectWithKeyValue() {
VariantObjectBuilder nested = arr.startObject();
nested.appendKey("nested");
nested.appendInt(1);
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() with an open nested object");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() while a nested object/array is still open.");
}

@Test
public void testOpenNestedArray() {
VariantBuilder b = new VariantBuilder();
VariantArrayBuilder arr = b.startArray();
arr.startArray();
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() with an open nested array");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() while a nested object/array is still open.");
}

@Test
Expand All @@ -283,11 +261,8 @@ public void testOpenNestedArrayWithElement() {
VariantArrayBuilder arr = b.startArray();
VariantArrayBuilder nested = arr.startArray();
nested.appendInt(1);
try {
b.endArray();
Assert.fail("Expected Exception when calling endArray() with an open nested array");
} catch (Exception e) {
// expected
}
assertThatThrownBy(b::endArray)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot call endArray() while a nested object/array is still open.");
}
}
Loading