From 87d636d8c6de0a1026c0375cd9953f0d781ecd7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 20 Sep 2019 12:09:02 +0200 Subject: [PATCH] [test] Add direct assertion method on RetainedDetector, polish few tests Tests that combined Awaitility, AssertJ and RetainedDetector now use the direct assertion of the later. SchedulersMetricsTest#shouldReportExecutionTimes now schedules with larger delays, so that the jitter is thinner. --- .../publisher/FluxBufferPredicateTest.java | 11 +++-------- .../core/publisher/FluxDistinctTest.java | 12 ++++++------ .../FluxDistinctUntilChangedTest.java | 12 ++++++------ .../publisher/FluxWindowPredicateTest.java | 11 +++-------- .../core/scheduler/SchedulersMetricsTest.java | 4 ++-- .../test/java/reactor/test/MemoryUtils.java | 18 +++++++++++++++++- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/reactor-core/src/test/java/reactor/core/publisher/FluxBufferPredicateTest.java b/reactor-core/src/test/java/reactor/core/publisher/FluxBufferPredicateTest.java index 4fc8070137..ce903cc1de 100644 --- a/reactor-core/src/test/java/reactor/core/publisher/FluxBufferPredicateTest.java +++ b/reactor-core/src/test/java/reactor/core/publisher/FluxBufferPredicateTest.java @@ -312,8 +312,7 @@ public void untilChangedDisposesStateOnComplete() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount(), - is(100L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -335,8 +334,7 @@ public void untilChangedDisposesStateOnError() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount(), - is(100L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -359,10 +357,7 @@ public void untilChangedDisposesStateOnCancel() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount(), - // 50 + 1 (taking 50 buffers requires having 51st element as a - // key in the predicate) - is(51L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test diff --git a/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctTest.java b/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctTest.java index e40ed27129..9eb91eda87 100644 --- a/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctTest.java +++ b/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctTest.java @@ -693,7 +693,7 @@ public void doesntRetainObjectsWithForcedCompleteOnSubscriber() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -719,7 +719,7 @@ public void doesntRetainObjectsWithForcedCompleteOnSubscriber_conditional() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -748,7 +748,7 @@ protected void hookOnError(Throwable throwable) { } await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -777,7 +777,7 @@ protected void hookOnError(Throwable throwable) { } await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -803,7 +803,7 @@ public void doesntRetainObjectsWithForcedCancelOnSubscriber() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -829,7 +829,7 @@ public void doesntRetainObjectsWithForcedCancelOnSubscriber_conditional() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isEqualTo(3)); + .untilAsserted(retainedDetector::assertAllFinalized); } static class DistinctDefault { diff --git a/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctUntilChangedTest.java b/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctUntilChangedTest.java index 381c6fdda5..ec80aeac2d 100644 --- a/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctUntilChangedTest.java +++ b/reactor-core/src/test/java/reactor/core/publisher/FluxDistinctUntilChangedTest.java @@ -377,7 +377,7 @@ public void doesntRetainObjectsWithForcedCompleteOnSubscriber() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -398,7 +398,7 @@ public void doesntRetainObjectsWithForcedCompleteOnSubscriber_conditional() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -422,7 +422,7 @@ protected void hookOnError(Throwable throwable) { } await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -446,7 +446,7 @@ protected void hookOnError(Throwable throwable) { } await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -467,7 +467,7 @@ public void doesntRetainObjectsWithForcedCancelOnSubscriber() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -488,7 +488,7 @@ public void doesntRetainObjectsWithForcedCancelOnSubscriber_conditional() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> assertThat(retainedDetector.finalizedCount()).isOne()); + .untilAsserted(retainedDetector::assertAllFinalized); } } diff --git a/reactor-core/src/test/java/reactor/core/publisher/FluxWindowPredicateTest.java b/reactor-core/src/test/java/reactor/core/publisher/FluxWindowPredicateTest.java index f863e7f1c4..6b3a2cf7ac 100644 --- a/reactor-core/src/test/java/reactor/core/publisher/FluxWindowPredicateTest.java +++ b/reactor-core/src/test/java/reactor/core/publisher/FluxWindowPredicateTest.java @@ -268,8 +268,7 @@ public void untilChangedDisposesStateOnComplete() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> Assert.assertThat(retainedDetector.finalizedCount(), - is(100L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -291,8 +290,7 @@ public void untilChangedDisposesStateOnError() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> Assert.assertThat(retainedDetector.finalizedCount(), - is(100L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Test @@ -315,10 +313,7 @@ public void untilChangedDisposesStateOnCancel() { await() .atMost(2, TimeUnit.SECONDS) - .untilAsserted(() -> Assert.assertThat(retainedDetector.finalizedCount(), - // 50 + 1 (taking 50 windows requires having 51st element as a - // key in the predicate) - is(51L))); + .untilAsserted(retainedDetector::assertAllFinalized); } @Override diff --git a/reactor-core/src/test/java/reactor/core/scheduler/SchedulersMetricsTest.java b/reactor-core/src/test/java/reactor/core/scheduler/SchedulersMetricsTest.java index 14f84345d8..107292ca1b 100644 --- a/reactor-core/src/test/java/reactor/core/scheduler/SchedulersMetricsTest.java +++ b/reactor-core/src/test/java/reactor/core/scheduler/SchedulersMetricsTest.java @@ -227,7 +227,7 @@ public void shouldReportExecutionTimes(Supplier schedulerSupplier, St Phaser phaser = new Phaser(1); for (int i = 1; i <= taskCount; i++) { phaser.register(); - int delay = i * 20; + int delay = i * 200; //bumped delay from 20ms to make actual scheduling times more precise scheduler.schedule(() -> { try { Thread.sleep(delay); @@ -250,7 +250,7 @@ public void shouldReportExecutionTimes(Supplier schedulerSupplier, St assertThat(timers.stream() .reduce(0d, (time, timer) -> time + timer.totalTime(TimeUnit.MILLISECONDS), Double::sum)) .as("total durations") - .isEqualTo(60 + 40 + 20, offset(10.0d)); + .isEqualTo(600 + 400 + 200, offset(20.0d)); assertThat(timers.stream().mapToLong(Timer::count).sum()) .as("count") .isEqualTo(taskCount); diff --git a/reactor-core/src/test/java/reactor/test/MemoryUtils.java b/reactor-core/src/test/java/reactor/test/MemoryUtils.java index 895df710cf..618cee041c 100644 --- a/reactor-core/src/test/java/reactor/test/MemoryUtils.java +++ b/reactor-core/src/test/java/reactor/test/MemoryUtils.java @@ -20,6 +20,9 @@ import java.lang.ref.ReferenceQueue; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.assertj.core.api.Assertions; /** * Test utility around memory, references, leaks and retained object detection. @@ -37,6 +40,7 @@ public static final class RetainedDetector { private final ReferenceQueue referenceQueue = new ReferenceQueue<>(); private final List> phantomReferences = new LinkedList<>(); private long finalizedSoFar = 0L; + private AtomicInteger trackedTotal = new AtomicInteger(0); /** * Track the object in this {@link RetainedDetector}'s {@link ReferenceQueue} @@ -47,6 +51,7 @@ public static final class RetainedDetector { */ public final T tracked(T object) { phantomReferences.add(new PhantomReference<>(object, referenceQueue)); + trackedTotal.incrementAndGet(); return object; } @@ -63,8 +68,19 @@ public final synchronized long finalizedCount() { return finalizedSoFar; } + /** + * @return the total number of objects that have been added to this {@link RetainedDetector} + */ public final long trackedTotal() { - return phantomReferences.size(); + return trackedTotal.get(); + } + + /** + * Assert that all tracked elements have been finalized. + * @throws AssertionError if some tracked elements have not been finalized + */ + public final void assertAllFinalized() { + Assertions.assertThat(this.finalizedCount()).as("all tracked finalized").isEqualTo(trackedTotal.get()); } }