nastra commented on code in PR #7655:
URL: https://github.com/apache/iceberg/pull/7655#discussion_r1211231496
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroDataWriter.java:
##########
@@ -90,23 +91,24 @@ public void testDataWriter() throws IOException {
DataFile dataFile = dataWriter.toDataFile();
- Assert.assertEquals("Format should be Avro", FileFormat.AVRO,
dataFile.format());
- Assert.assertEquals("Should be data file", FileContent.DATA,
dataFile.content());
- Assert.assertEquals("Record count should match", records.size(),
dataFile.recordCount());
- Assert.assertEquals("Partition should be empty", 0,
dataFile.partition().size());
- Assert.assertEquals(
- "Sort order should match", sortOrder.orderId(), (int)
dataFile.sortOrderId());
- Assert.assertNull("Key metadata should be null", dataFile.keyMetadata());
+ assertThat(dataFile.format()).as("Format should be
Avro").isEqualTo(FileFormat.AVRO);
+ assertThat(dataFile.content()).as("Should be data
file").isEqualTo(FileContent.DATA);
+ assertThat(dataFile.recordCount()).as("Record count should
match").isEqualTo(records.size());
+ assertThat(dataFile.partition().size()).as("Partition should be
empty").isEqualTo(0);
+ assertThat((int) dataFile.sortOrderId())
Review Comment:
I think we can remove the casting to `int` here
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroEnums.java:
##########
@@ -53,15 +53,15 @@ public void writeAndValidateEnums() throws IOException {
.endRecord();
org.apache.avro.Schema enumSchema =
avroSchema.getField("enumCol").schema().getTypes().get(0);
- Record enumRecord1 = new GenericData.Record(avroSchema);
+ Record enumRecord1 = new Record(avroSchema);
Review Comment:
are these changes needed?
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroDeleteWriters.java:
##########
@@ -56,9 +56,9 @@ public class TestAvroDeleteWriters {
private List<Record> records;
- @Rule public TemporaryFolder temp = new TemporaryFolder();
+ @TempDir java.nio.file.Path temp;
Review Comment:
do we need the fully qualified name here? I think we can just add an import
here and in the other files
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroFileSplit.java:
##########
@@ -113,16 +112,17 @@ public void testPosField() throws IOException {
List<Record> records = readAvro(file, projection, 0, file.getLength());
for (int i = 0; i < expected.size(); i += 1) {
- Assert.assertEquals(
- "Field _pos should match",
- (long) i,
- records.get(i).getField(MetadataColumns.ROW_POSITION.name()));
- Assert.assertEquals(
- "Field id should match", expected.get(i).getField("id"),
records.get(i).getField("id"));
- Assert.assertEquals(
- "Field data should match",
- expected.get(i).getField("data"),
- records.get(i).getField("data"));
+ assertThat(records.get(i).getField(MetadataColumns.ROW_POSITION.name()))
+ .as("Field _pos should match")
+ .isEqualTo((long) i);
Review Comment:
I believe we should be able to remove the casting here
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroFileSplit.java:
##########
@@ -136,42 +136,37 @@ public void testPosFieldWithSplits() throws IOException {
List<Record> secondHalf =
readAvro(file, projection, splitLocation + 1, end - splitLocation - 1);
- Assert.assertNotEquals("Second split should not be empty", 0,
secondHalf.size());
+ assertThat(secondHalf.size()).as("Second split should not be
empty").isNotEqualTo(0);
List<Record> firstHalf = readAvro(file, projection, 0, splitLocation);
- Assert.assertNotEquals("First split should not be empty", 0,
firstHalf.size());
+ assertThat(firstHalf.size()).as("First split should not be
empty").isNotEqualTo(0);
- Assert.assertEquals(
- "Total records should match expected",
- expected.size(),
- firstHalf.size() + secondHalf.size());
+ assertThat(firstHalf.size() + secondHalf.size())
+ .as("Total records should match expected")
+ .isEqualTo(expected.size());
for (int i = 0; i < firstHalf.size(); i += 1) {
- Assert.assertEquals(
- "Field _pos should match",
- (long) i,
- firstHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()));
- Assert.assertEquals(
- "Field id should match", expected.get(i).getField("id"),
firstHalf.get(i).getField("id"));
- Assert.assertEquals(
- "Field data should match",
- expected.get(i).getField("data"),
- firstHalf.get(i).getField("data"));
+
assertThat(firstHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()))
+ .as("Field _pos should match")
+ .isEqualTo((long) i);
Review Comment:
casting can be removed
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroFileSplit.java:
##########
@@ -136,42 +136,37 @@ public void testPosFieldWithSplits() throws IOException {
List<Record> secondHalf =
readAvro(file, projection, splitLocation + 1, end - splitLocation - 1);
- Assert.assertNotEquals("Second split should not be empty", 0,
secondHalf.size());
+ assertThat(secondHalf.size()).as("Second split should not be
empty").isNotEqualTo(0);
List<Record> firstHalf = readAvro(file, projection, 0, splitLocation);
- Assert.assertNotEquals("First split should not be empty", 0,
firstHalf.size());
+ assertThat(firstHalf.size()).as("First split should not be
empty").isNotEqualTo(0);
- Assert.assertEquals(
- "Total records should match expected",
- expected.size(),
- firstHalf.size() + secondHalf.size());
+ assertThat(firstHalf.size() + secondHalf.size())
+ .as("Total records should match expected")
+ .isEqualTo(expected.size());
for (int i = 0; i < firstHalf.size(); i += 1) {
- Assert.assertEquals(
- "Field _pos should match",
- (long) i,
- firstHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()));
- Assert.assertEquals(
- "Field id should match", expected.get(i).getField("id"),
firstHalf.get(i).getField("id"));
- Assert.assertEquals(
- "Field data should match",
- expected.get(i).getField("data"),
- firstHalf.get(i).getField("data"));
+
assertThat(firstHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()))
+ .as("Field _pos should match")
+ .isEqualTo((long) i);
+ assertThat(firstHalf.get(i).getField("id"))
+ .as("Field id should match")
+ .isEqualTo(expected.get(i).getField("id"));
+ assertThat(firstHalf.get(i).getField("data"))
+ .as("Field data should match")
+ .isEqualTo(expected.get(i).getField("data"));
}
for (int i = 0; i < secondHalf.size(); i += 1) {
- Assert.assertEquals(
- "Field _pos should match",
- (long) (firstHalf.size() + i),
- secondHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()));
- Assert.assertEquals(
- "Field id should match",
- expected.get(firstHalf.size() + i).getField("id"),
- secondHalf.get(i).getField("id"));
- Assert.assertEquals(
- "Field data should match",
- expected.get(firstHalf.size() + i).getField("data"),
- secondHalf.get(i).getField("data"));
+
assertThat(secondHalf.get(i).getField(MetadataColumns.ROW_POSITION.name()))
+ .as("Field _pos should match")
+ .isEqualTo((long) (firstHalf.size() + i));
Review Comment:
casting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
