nastra commented on code in PR #7655:
URL: https://github.com/apache/iceberg/pull/7655#discussion_r1212717781
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroEnums.java:
##########
@@ -71,8 +72,8 @@ public void writeAndValidateEnums() throws IOException {
}
Schema schema = new
Schema(AvroSchemaUtil.convert(avroSchema).asStructType().fields());
- List<GenericData.Record> rows;
- try (AvroIterable<GenericData.Record> reader =
+ List<Record> rows;
+ try (AvroIterable<Record> reader =
Review Comment:
nit: I think it would generally be good to not apply changes that aren't
related to Junit5 migration
##########
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 is still present
##########
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:
looks like the casting is still present, do we need it?
##########
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 is still present
--
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]
