From bfa8fd0b1f467e2629cf2f99811bfaad15030e2d Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 11:39:57 +0200 Subject: [PATCH] Make tests pass after removing `acquire` from the public API --- buffer-memseg/pom.xml | 51 ++++--- buffer-tests/pom.xml | 1 - .../api/tests/BufferCompositionTest.java | 144 +++++------------- .../api/tests/BufferEnsureWritableTest.java | 17 --- .../tests/BufferReferenceCountingTest.java | 53 ------- pom.xml | 1 + 6 files changed, 67 insertions(+), 200 deletions(-) diff --git a/buffer-memseg/pom.xml b/buffer-memseg/pom.xml index 16d8116..3bd4f08 100644 --- a/buffer-memseg/pom.xml +++ b/buffer-memseg/pom.xml @@ -15,6 +15,34 @@ Netty/Incubator/Buffer MemorySegment jar + + + + maven-compiler-plugin + 3.8.1 + + ${java.version} + true + ${java.compatibility} + ${java.compatibility} + ${java.version} + true + true + true + true + -Xlint:-options + 256m + 1024m + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + io.netty.incubator @@ -42,27 +70,4 @@ test - - - - - maven-compiler-plugin - 3.8.1 - - ${java.version} - true - ${java.compatibility} - ${java.compatibility} - ${java.version} - true - true - true - true - -Xlint:-options - 256m - 1024m - - - - \ No newline at end of file diff --git a/buffer-tests/pom.xml b/buffer-tests/pom.xml index 78c2cce..66c5707 100644 --- a/buffer-tests/pom.xml +++ b/buffer-tests/pom.xml @@ -31,7 +31,6 @@ jar - 3.0.0-M5 false -XX:+HeapDumpOnOutOfMemoryError diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java index c57e191..a6d3715 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java @@ -29,47 +29,21 @@ import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class BufferCompositionTest extends BufferTestSupport { - @Test - public void compositeBufferCanOnlyBeOwnedWhenAllConstituentBuffersAreOwned() { - try (BufferAllocator allocator = BufferAllocator.heap()) { - var composite = asRS(CompositeBuffer.compose(allocator)); - try (var a = asRS(allocator.allocate(8))) { - assertTrue(a.isOwned()); - Buffer leakB; - try (var b = asRS(allocator.allocate(8))) { - assertTrue(a.isOwned()); - assertTrue(b.isOwned()); - composite = asRS(CompositeBuffer.compose(allocator, a.send(), b.send())); - assertFalse(composite.isOwned()); - assertFalse(a.isOwned()); - assertFalse(b.isOwned()); - leakB = b; - } - assertFalse(composite.isOwned()); - assertFalse(a.isOwned()); - assertTrue(asRS(leakB).isOwned()); - } - assertTrue(composite.isOwned()); - } - } - @Test public void compositeBuffersCannotHaveDuplicateComponents() { try (BufferAllocator allocator = BufferAllocator.heap()) { Send a = allocator.allocate(4).send(); - var e = assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, a)); - assertThat(e).hasMessageContaining("duplicate"); + var e = assertThrows(IllegalStateException.class, () -> CompositeBuffer.compose(allocator, a, a)); + assertThat(e).hasMessageContaining("already been received"); Send b = allocator.allocate(4).send(); try (CompositeBuffer composite = CompositeBuffer.compose(allocator, b)) { - e = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); - assertThat(e).hasMessageContaining("duplicate"); + e = assertThrows(IllegalStateException.class, () -> composite.extendWith(b)); + assertThat(e).hasMessageContaining("already been received"); } } } @@ -89,28 +63,20 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void compositeBufferMustNotBeAllowedToContainThemselves() { try (BufferAllocator allocator = BufferAllocator.heap()) { - Buffer a = allocator.allocate(4); - CompositeBuffer buf = CompositeBuffer.compose(allocator, a.send()); - try (buf; a) { - a.close(); - try { - assertThrows(IllegalArgumentException.class, () -> buf.extendWith(buf.send())); - assertTrue(buf.isOwned()); - try (Buffer composite = CompositeBuffer.compose(allocator, buf.send())) { - // the composing increments the reference count of constituent buffers... - // counter-act this, so it can be extended: - a.close(); // buf is now owned, so it can be extended. - try { - assertThrows(IllegalArgumentException.class, - () -> buf.extendWith(composite.send())); - } finally { - asRS(a).acquire(); // restore the reference count to align with our try-with-resources structure. - } - } - assertTrue(buf.isOwned()); - } finally { - asRS(a).acquire(); - } + CompositeBuffer bufA = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); + Send sendA = bufA.send(); + try { + assertThrows(IllegalStateException.class, () -> bufA.extendWith(sendA)); + } finally { + sendA.discard(); + } + + CompositeBuffer bufB = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); + Send sendB = bufB.send(); + try (CompositeBuffer compositeBuffer = CompositeBuffer.compose(allocator, sendB)) { + assertThrows(IllegalStateException.class, () -> compositeBuffer.extendWith(sendB)); + } finally { + sendB.discard(); } } } @@ -182,7 +148,7 @@ public class BufferCompositionTest extends BufferTestSupport { composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { - var exc = assertThrows(IllegalArgumentException.class, + var exc = assertThrows(IllegalStateException.class, () -> composite.extendWith(composite.send())); assertThat(exc).hasMessageContaining("cannot be extended"); } @@ -385,27 +351,25 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void whenExtendingCompositeBufferWithReadOffsetLessThanCapacityExtensionReadOffsetMustZero() { - try (BufferAllocator allocator = BufferAllocator.heap()) { - CompositeBuffer composite; - try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a.send()); - } - try (composite) { - composite.writeLong(0); - composite.readInt(); - try (Buffer b = allocator.allocate(8)) { - b.writeInt(1); - b.readInt(); - var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b.send())); - assertThat(exc).hasMessageContaining("unread gap"); - b.readerOffset(0); - composite.extendWith(b.send()); - assertThat(composite.capacity()).isEqualTo(16); - assertThat(composite.writerOffset()).isEqualTo(12); - assertThat(composite.readerOffset()).isEqualTo(4); - } - } + try (BufferAllocator allocator = BufferAllocator.heap(); + CompositeBuffer composite = CompositeBuffer.compose(allocator, allocator.allocate(8).send())) { + composite.writeLong(0); + composite.readInt(); + + Buffer b = allocator.allocate(8); + b.writeInt(1); + b.readInt(); + var exc = assertThrows(IllegalArgumentException.class, + () -> composite.extendWith(b.send())); + assertThat(exc).hasMessageContaining("unread gap"); + assertThat(composite.capacity()).isEqualTo(8); + assertThat(composite.writerOffset()).isEqualTo(8); + assertThat(composite.readerOffset()).isEqualTo(4); + + composite.extendWith(allocator.allocate(8).writeInt(1).send()); + assertThat(composite.capacity()).isEqualTo(16); + assertThat(composite.writerOffset()).isEqualTo(12); + assertThat(composite.readerOffset()).isEqualTo(4); } } @@ -481,38 +445,6 @@ public class BufferCompositionTest extends BufferTestSupport { } } - @Test - public void splitComponentsFloorMustThrowIfCompositeBufferIsNotOwned() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(8); - Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(0)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(4)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(7)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(8)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(9)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(12)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(16)); - } - } - - @Test - public void splitComponentsCeilMustThrowIfCompositeBufferIsNotOwned() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(8); - Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(0)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(4)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(7)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(8)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(9)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(12)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(16)); - } - } - @Test public void splitComponentsFloorMustThrowOnOutOfBounds() { try (BufferAllocator allocator = BufferAllocator.heap(); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java index 8be646c..d301daf 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java @@ -26,23 +26,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; public class BufferEnsureWritableTest extends BufferTestSupport { - - @ParameterizedTest - @MethodSource("allocators") - public void ensureWritableMustThrowForBorrowedBuffers(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - try (Buffer slice = buf.slice()) { - assertThrows(IllegalStateException.class, () -> slice.ensureWritable(1)); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); - } - try (Buffer compose = CompositeBuffer.compose(allocator, buf.send())) { - assertThrows(IllegalStateException.class, () -> compose.ensureWritable(1)); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); - } - } - } - @ParameterizedTest @MethodSource("allocators") public void ensureWritableMustThrowForNegativeSize(Fixture fixture) { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index 8d89a82..16fff44 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -296,59 +296,6 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } } - @ParameterizedTest - @MethodSource("nonCompositeAllocators") - public void acquireComposingAndSlicingMustIncrementBorrows(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - int borrows = countBorrows(buf); - try (Buffer ignored = asRS(buf).acquire()) { - assertEquals(borrows + 1, countBorrows(buf)); - try (Buffer slice = buf.slice()) { - assertEquals(0, slice.capacity()); // We haven't written anything, so the slice is empty. - int sliceBorrows = countBorrows(slice); - assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { - assertEquals(borrows + 3, countBorrows(buf)); - // Note: Slice is empty; not acquired by the composite buffer. - assertEquals(sliceBorrows, countBorrows(slice)); - } - assertEquals(sliceBorrows, countBorrows(slice)); - assertEquals(borrows + 2, countBorrows(buf)); - } - assertEquals(borrows + 1, countBorrows(buf)); - } - assertEquals(borrows, countBorrows(buf)); - } - } - - @ParameterizedTest - @MethodSource("nonCompositeAllocators") - public void acquireComposingAndSlicingMustIncrementBorrowsWithData(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - buf.writeByte((byte) 1); - int borrows = countBorrows(buf); - try (Buffer ignored = asRS(buf).acquire()) { - assertEquals(borrows + 1, countBorrows(buf)); - try (Buffer slice = buf.slice()) { - assertEquals(1, slice.capacity()); - int sliceBorrows = countBorrows(slice); - assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { - assertEquals(borrows + 3, countBorrows(buf)); - assertEquals(sliceBorrows + 1, countBorrows(slice)); - } - assertEquals(sliceBorrows, countBorrows(slice)); - assertEquals(borrows + 2, countBorrows(buf)); - } - assertEquals(borrows + 1, countBorrows(buf)); - } - assertEquals(borrows, countBorrows(buf)); - assertTrue(asRS(buf).isOwned()); - } - } - @ParameterizedTest @MethodSource("allocators") public void sliceMustBecomeOwnedOnSourceBufferClose(Fixture fixture) { diff --git a/pom.xml b/pom.xml index 2e98b93..2dbf0e2 100644 --- a/pom.xml +++ b/pom.xml @@ -75,6 +75,7 @@ 11 5.7.0 + 3.0.0-M5