You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2023/06/06 08:12:37 UTC
[GitHub] [iceberg] nastra commented on a diff in pull request #7767: Core: Switch tests to Junit5 in catalog,encryption,inmemory,io,view
nastra commented on code in PR #7767:
URL: https://github.com/apache/iceberg/pull/7767#discussion_r1219143741
##########
core/src/test/java/org/apache/iceberg/io/TestMultiBufferInputStream.java:
##########
@@ -62,62 +63,66 @@ public void testSliceData() throws Exception {
buffers.add(stream.slice(bytesToSlice));
}
- Assert.assertEquals("Position should be at end", length, stream.getPos());
- Assert.assertEquals("Should produce 5 buffers", 5, buffers.size());
+ assertThat(stream.getPos()).as("Position should be at end").isEqualTo(length);
+ assertThat(buffers.size()).as("Should produce 5 buffers").isEqualTo(5);
int i = 0;
// one is a view of the first buffer because it is smaller
ByteBuffer one = buffers.get(0);
- Assert.assertSame("Should be a duplicate of the first array", one.array(), DATA.get(0).array());
- Assert.assertEquals(8, one.remaining());
- Assert.assertEquals(0, one.position());
- Assert.assertEquals(8, one.limit());
- Assert.assertEquals(9, one.capacity());
+ assertThat(DATA.get(0).array())
+ .as("Should be a duplicate of the first array")
+ .isSameAs(one.array());
+ assertThat(one.remaining()).isEqualTo(8);
+ assertThat(one.position()).isEqualTo(0);
+ assertThat(one.limit()).isEqualTo(8);
+ assertThat(one.capacity()).isEqualTo(9);
for (; i < 8; i += 1) {
- Assert.assertEquals("Should produce correct values", i, one.get());
+ assertThat(one.get()).as("Should produce correct values").isEqualTo((byte) i);
}
// two should be a copy of the next 8 bytes
ByteBuffer two = buffers.get(1);
- Assert.assertEquals(8, two.remaining());
- Assert.assertEquals(0, two.position());
- Assert.assertEquals(8, two.limit());
- Assert.assertEquals(8, two.capacity());
+ assertThat(two.remaining()).isEqualTo(8);
+ assertThat(two.position()).isEqualTo(0);
+ assertThat(two.limit()).isEqualTo(8);
+ assertThat(two.capacity()).isEqualTo(8);
for (; i < 16; i += 1) {
- Assert.assertEquals("Should produce correct values", i, two.get());
+ assertThat(two.get()).as("Should produce correct values").isEqualTo((byte) i);
}
// three is a copy of part of the 4th buffer
ByteBuffer three = buffers.get(2);
- Assert.assertSame(
- "Should be a duplicate of the fourth array", three.array(), DATA.get(3).array());
- Assert.assertEquals(8, three.remaining());
- Assert.assertEquals(3, three.position());
- Assert.assertEquals(11, three.limit());
- Assert.assertEquals(12, three.capacity());
+ assertThat(DATA.get(3).array())
+ .as("Should be a duplicate of the fourth array")
+ .isSameAs(three.array());
+ assertThat(three.remaining()).isEqualTo(8);
+ assertThat(three.position()).isEqualTo(3);
+ assertThat(three.limit()).isEqualTo(11);
+ assertThat(three.capacity()).isEqualTo(12);
for (; i < 24; i += 1) {
- Assert.assertEquals("Should produce correct values", i, three.get());
+ assertThat(three.get()).as("Should produce correct values").isEqualTo((byte) i);
}
// four should be a copy of the next 8 bytes
ByteBuffer four = buffers.get(3);
- Assert.assertEquals(8, four.remaining());
- Assert.assertEquals(0, four.position());
- Assert.assertEquals(8, four.limit());
- Assert.assertEquals(8, four.capacity());
+ assertThat(four.remaining()).isEqualTo(8);
+ assertThat(four.position()).isEqualTo(0);
+ assertThat(four.limit()).isEqualTo(8);
+ assertThat(four.capacity()).isEqualTo(8);
for (; i < 32; i += 1) {
- Assert.assertEquals("Should produce correct values", i, four.get());
+ assertThat(four.get()).as("Should produce correct values").isEqualTo((byte) i);
}
// five should be a copy of the next 8 bytes
ByteBuffer five = buffers.get(4);
- Assert.assertEquals(3, five.remaining());
- Assert.assertEquals(0, five.position());
- Assert.assertEquals(3, five.limit());
- Assert.assertEquals(3, five.capacity());
+
Review Comment:
nit: unnecessary newline
##########
core/src/test/java/org/apache/iceberg/inmemory/TestInMemoryInputFile.java:
##########
@@ -22,16 +22,17 @@
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import org.assertj.core.api.Assertions;
-import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
public class TestInMemoryInputFile {
@Test
public void testReadAfterClose() throws IOException {
InMemoryInputFile inputFile =
new InMemoryInputFile("abc".getBytes(StandardCharsets.ISO_8859_1));
InputStream inputStream = inputFile.newStream();
- Assert.assertEquals('a', inputStream.read());
+ Assertions.assertThat(inputStream.read())
+ .as("Checking read value from inputStream")
Review Comment:
do we really need this description?
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -168,70 +166,71 @@ protected boolean supportsNamesWithSlashes() {
public void testCreateNamespace() {
C catalog = catalog();
- Assert.assertFalse("Namespace should not exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should not exist").isFalse();
catalog.createNamespace(NS);
- Assert.assertTrue(
- "Catalog should have the created namespace", catalog.listNamespaces().contains(NS));
- Assert.assertTrue("Namespace should exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.listNamespaces())
+ .as("Catalog should have the created namespace")
+ .contains(NS);
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should exist").isTrue();
}
@Test
public void testCreateExistingNamespace() {
C catalog = catalog();
- Assert.assertFalse("Namespace should not exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should not exist").isFalse();
catalog.createNamespace(NS);
- Assert.assertTrue("Namespace should exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should exist").isTrue();
Assertions.assertThatThrownBy(() -> catalog.createNamespace(NS))
.isInstanceOf(AlreadyExistsException.class)
.hasMessageContaining("Namespace already exists");
- Assert.assertTrue("Namespace should still exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should still exist").isTrue();
}
@Test
public void testCreateNamespaceWithProperties() {
- Assume.assumeTrue(supportsNamespaceProperties());
+ Assumptions.assumeTrue(supportsNamespaceProperties());
C catalog = catalog();
- Assert.assertFalse("Namespace should not exist", catalog.namespaceExists(NS));
+ Assertions.assertThat(catalog.namespaceExists(NS)).as("Namespace should not exist").isFalse();
Map<String, String> createProps = ImmutableMap.of("prop", "val");
catalog.createNamespace(NS, createProps);
- Assert.assertTrue("Namespace should exist", catalog.namespaceExists(NS));
+
Review Comment:
unnecessary newline
##########
core/src/test/java/org/apache/iceberg/io/TestByteBufferInputStreams.java:
##########
@@ -506,7 +562,8 @@ public void testMarkDoubleReset() throws Exception {
ByteBufferInputStream stream = newStream();
stream.mark(5);
- Assert.assertEquals("Should read 5 bytes", 5, stream.read(new byte[5]));
+
Review Comment:
unnecessary newline
##########
core/src/test/java/org/apache/iceberg/io/TestSingleBufferInputStream.java:
##########
@@ -40,8 +41,9 @@ protected ByteBufferInputStream newStream() {
@Override
protected void checkOriginalData() {
- Assert.assertEquals("Position should not change", 0, DATA.position());
- Assert.assertEquals("Limit should not change", DATA.array().length, DATA.limit());
+ assertThat(DATA.position()).as("Position should not change").isEqualTo(0);
+
Review Comment:
nit: unnecessary newline
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org