This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit d17ebf34c0cd3732e4e0c8fde0e6f89606ab3bcc Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Tue Feb 20 10:59:50 2024 +0100 Add a common base class for tests that depend on the Garbage Collector activity. Those tests are executed in isolation for making them a little bit less undeterministic. --- .../org/apache/sis/test/TestConfiguration.java | 11 -- .../test/org/apache/sis/test/TestUtilities.java | 68 ------------ .../org/apache/sis/util/collection/CacheTest.java | 7 +- .../apache/sis/util/collection/RangeSetTest.java | 2 - .../apache/sis/util/collection/TestCaseWithGC.java | 114 +++++++++++++++++++++ .../sis/util/collection/WeakHashSetTest.java | 15 ++- .../sis/util/collection/WeakValueHashMapTest.java | 7 +- 7 files changed, 124 insertions(+), 100 deletions(-) diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestConfiguration.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestConfiguration.java index 99f869a629..8b2e92012b 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestConfiguration.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestConfiguration.java @@ -58,15 +58,4 @@ public final class TestConfiguration extends Static { */ private TestConfiguration() { } - - /** - * Returns {@code true} if tests that may depend on the garbage collector activity are allowed. - * Those tests are a little bit dangerous since they may randomly fail on a server too busy for - * running the garbage collector as fast as expected. - * - * @return {@code true} if tests that may depend on garbage collector activity are allowed. - */ - public static boolean allowGarbageCollectorDependentTests() { - return true; - } } diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestUtilities.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestUtilities.java index b929a5181e..464ad86ba8 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestUtilities.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/test/TestUtilities.java @@ -21,7 +21,6 @@ import java.util.Locale; import java.util.TimeZone; import java.util.Iterator; import java.util.Random; -import java.util.concurrent.Callable; import java.io.PrintWriter; import java.io.IOException; import java.io.InputStream; @@ -62,14 +61,6 @@ public final class TestUtilities extends Static { */ private static final int SEPARATOR_WIDTH = 80; - /** - * Maximal time that {@code waitFoo()} methods can wait, in milliseconds. - * - * @see #waitForBlockedState(Thread) - * @see #waitForGarbageCollection(Callable) - */ - private static final int MAXIMAL_WAIT_TIME = 1000; - /** * Date parser and formatter using the {@code "yyyy-MM-dd HH:mm:ss"} pattern * and UTC time zone. @@ -396,65 +387,6 @@ public final class TestUtilities extends Static { } } - /** - * Waits up to one second for the given thread to reach the - * {@linkplain java.lang.Thread.State#BLOCKED blocked} or the - * {@linkplain java.lang.Thread.State#WAITING waiting} state. - * - * @param thread the thread to wait for blocked or waiting state. - * @throws IllegalThreadStateException if the thread has terminated its execution, - * or has not reached the waiting or blocked state before the timeout. - * @throws InterruptedException if this thread has been interrupted while waiting. - */ - public static void waitForBlockedState(final Thread thread) throws IllegalThreadStateException, InterruptedException { - int retry = MAXIMAL_WAIT_TIME / 5; // 5 shall be the same number as in the call to Thread.sleep. - do { - Thread.sleep(5); - switch (thread.getState()) { - case WAITING: - case BLOCKED: return; - case TERMINATED: throw new IllegalThreadStateException("The thread has completed execution."); - } - } while (--retry != 0); - throw new IllegalThreadStateException("The thread is not in a blocked or waiting state."); - } - - /** - * Waits up to one second for the garbage collector to do its work. This method can be invoked - * only if {@link TestConfiguration#allowGarbageCollectorDependentTests()} returns {@code true}. - * - * <p>Note that this method does not throw any exception if the given condition has not been - * reached before the timeout. Instead, it is the caller responsibility to test the return - * value. This method is designed that way because the caller can usually produce a more - * accurate error message about which value has not been garbage collected as expected.</p> - * - * @param stopCondition a condition which return {@code true} if this method can stop waiting, - * or {@code false} if it needs to ask again for garbage collection. - * @return {@code true} if the given condition has been met, or {@code false} if we waited up - * to the timeout without meeting the given condition. - * @throws InterruptedException if this thread has been interrupted while waiting. - */ - public static boolean waitForGarbageCollection(final Callable<Boolean> stopCondition) throws InterruptedException { - assertTrue(TestConfiguration.allowGarbageCollectorDependentTests(), "GC-dependent tests not allowed in this run."); - int retry = MAXIMAL_WAIT_TIME / 50; // 50 shall be the same number as in the call to Thread.sleep. - boolean stop; - do { - if (--retry == 0) { - return false; - } - Thread.sleep(50); - System.gc(); - try { - stop = stopCondition.call(); - } catch (RuntimeException e) { - throw e; - } catch (Exception e) { - throw new AssertionError(e); - } - } while (!stop); - return true; - } - /** * Copies the full content of the given input stream in a temporary file and returns the channel for that file. * The file is opened with {@link StandardOpenOption#DELETE_ON_CLOSE}, together with read and write options. diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/CacheTest.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/CacheTest.java index a2e120f7fd..3335ad6953 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/CacheTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/CacheTest.java @@ -34,9 +34,7 @@ import org.apache.sis.util.internal.StandardDateFormat; // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; -import org.junit.jupiter.api.parallel.Isolated; import org.apache.sis.test.TestUtilities; -import org.apache.sis.test.TestCase; import org.apache.sis.test.Performance; import static org.apache.sis.test.Assertions.assertMapEquals; @@ -46,8 +44,7 @@ import static org.apache.sis.test.Assertions.assertMapEquals; * * @author Martin Desruisseaux (Geomatys) */ -@Isolated("Depends on garbage collector activity") -public final class CacheTest extends TestCase { +public final class CacheTest extends TestCaseWithGC { /** * Creates a new test case. */ @@ -164,7 +161,7 @@ public final class CacheTest extends TestCase { assertTrue(handler instanceof Cache<?,?>.Work); final OtherThread thread = new OtherThread(); thread.start(); - TestUtilities.waitForBlockedState(thread); + waitForBlockedState(thread); assertNull(cache.peek(keyByOtherThread), "The blocked thread shall not have added a value."); /* * Write. This will release the lock and let the other thread continue its job. diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/RangeSetTest.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/RangeSetTest.java index 814dc234f6..08a006d0f8 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/RangeSetTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/RangeSetTest.java @@ -23,7 +23,6 @@ import java.util.Random; import java.util.Arrays; import java.util.Collections; import java.util.SortedSet; -import java.io.PrintWriter; import org.apache.sis.measure.Range; import org.apache.sis.measure.NumberRange; import static org.apache.sis.util.internal.StandardDateFormat.MILLISECONDS_PER_DAY; @@ -575,7 +574,6 @@ public final class RangeSetTest extends TestCase { */ @Performance public void stress() throws InterruptedException { - final PrintWriter out = TestCase.out; final Random r = TestUtilities.createRandomNumberGenerator(); for (int p=0; p<10; p++) { final long start = System.nanoTime(); diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/TestCaseWithGC.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/TestCaseWithGC.java new file mode 100644 index 0000000000..ef24870f74 --- /dev/null +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/TestCaseWithGC.java @@ -0,0 +1,114 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sis.util.collection; + +import java.util.concurrent.Callable; + +// Test dependencies +import static org.junit.jupiter.api.Assertions.assertTrue; +import org.junit.jupiter.api.parallel.Isolated; +import org.apache.sis.test.TestCase; + + +/** + * Base class of tests that may depend on the garbage collector activity. + * + * @author Martin Desruisseaux (Geomatys) + */ +@Isolated("Depends on garbage collector activity") +abstract class TestCaseWithGC extends TestCase { + /** + * Whether the tests that may depend on the garbage collector activity are allowed. + * Those tests are a little bit dangerous since they may randomly fail on a server + * too busy for running the garbage collector as fast as expected. + */ + static final boolean GC_DEPENDENT_TESTS_ENABLED = true; + + /** + * Maximal time that {@code waitFoo()} methods can wait, in milliseconds. + * + * @see #waitForBlockedState(Thread) + * @see #waitForGarbageCollection(Callable) + */ + private static final int MAXIMAL_WAIT_TIME = 1000; + + /** + * Creates a new test case. + */ + protected TestCaseWithGC() { + } + + /** + * Waits up to one second for the garbage collector to do its work. + * This method can be invoked only if {@link #GC_DEPENDENT_TESTS_ENABLED} is {@code true}. + * + * <p>Note that this method does not throw any exception if the given condition has not been + * reached before the timeout. Instead, it is the caller responsibility to test the return value. + * This method is designed that way because the caller can usually produce a more accurate error + * message about which value has not been garbage collected as expected.</p> + * + * @param stopCondition a condition which return {@code true} if this method can stop waiting, + * or {@code false} if it needs to ask again for garbage collection. + * @return {@code true} if the given condition has been met, or {@code false} if we waited up + * to the timeout without meeting the given condition. + * @throws InterruptedException if this thread has been interrupted while waiting. + */ + static boolean waitForGarbageCollection(final Callable<Boolean> stopCondition) throws InterruptedException { + assertTrue(GC_DEPENDENT_TESTS_ENABLED, "GC-dependent tests not allowed in this run."); + int retry = MAXIMAL_WAIT_TIME / 50; // 50 shall be the same number as in the call to Thread.sleep. + boolean stop; + do { + if (--retry == 0) { + return false; + } + Thread.sleep(50); + System.gc(); + try { + stop = stopCondition.call(); + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new AssertionError(e); + } + } while (!stop); + return true; + } + + /** + * Waits up to one second for the given thread to reach the blocked or the waiting state. + * + * @param thread the thread to wait for blocked or waiting state. + * @throws IllegalThreadStateException if the thread has terminated its execution, + * or has not reached the waiting or blocked state before the timeout. + * @throws InterruptedException if this thread has been interrupted while waiting. + * + * @see java.lang.Thread.State#BLOCKED + * @see java.lang.Thread.State#WAITING + */ + static void waitForBlockedState(final Thread thread) throws IllegalThreadStateException, InterruptedException { + int retry = MAXIMAL_WAIT_TIME / 5; // 5 shall be the same number as in the call to Thread.sleep. + do { + Thread.sleep(5); + switch (thread.getState()) { + case WAITING: + case BLOCKED: return; + case TERMINATED: throw new IllegalThreadStateException("The thread has completed execution."); + } + } while (--retry != 0); + throw new IllegalThreadStateException("The thread is not in a blocked or waiting state."); + } +} diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakHashSetTest.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakHashSetTest.java index 7275a5d7e5..04071b1c9a 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakHashSetTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakHashSetTest.java @@ -22,10 +22,7 @@ import java.util.Random; // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; -import org.apache.sis.test.TestCase; -import org.apache.sis.test.TestConfiguration; import static org.apache.sis.test.Assertions.assertSetEquals; -import static org.apache.sis.test.TestUtilities.waitForGarbageCollection; /** @@ -34,7 +31,7 @@ import static org.apache.sis.test.TestUtilities.waitForGarbageCollection; * * @author Martin Desruisseaux (IRD, Geomatys) */ -public final class WeakHashSetTest extends TestCase { +public final class WeakHashSetTest extends TestCaseWithGC { /** * The size of the test sets to be created. */ @@ -149,12 +146,12 @@ public final class WeakHashSetTest extends TestCase { assertTrue(weakSet.containsAll(strongSet), "containsAll:"); } /* - * The test below needs the garbage collector to complete fully its job in a timely - * manner. A failure in those tests is not necessarily a WeakValueHashMap bug, as it - * could be caused by a heavy server load preventing GC to complete its work. If this - * happen too often, we may turn off the "allow garbage collector dependent tests" flag. + * The test below needs the garbage collector to complete fully its job in a timely manner. + * A failure in those tests is not necessarily a WeakValueHashMap bug, as it could be caused + * by a heavy server load preventing GC to complete its work. If this happen too often, + * we may turn off the "allow garbage collector dependent tests" flag. */ - if (TestConfiguration.allowGarbageCollectorDependentTests()) { + if (GC_DEPENDENT_TESTS_ENABLED) { waitForGarbageCollection(() -> weakSet.size() == strongSet.size()); assertSetEquals(strongSet, weakSet); /* diff --git a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakValueHashMapTest.java b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakValueHashMapTest.java index a3b42a8284..f5ad861fd7 100644 --- a/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakValueHashMapTest.java +++ b/endorsed/src/org.apache.sis.util/test/org/apache/sis/util/collection/WeakValueHashMapTest.java @@ -23,11 +23,8 @@ import java.util.Random; // Test dependencies import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.*; -import org.apache.sis.test.TestCase; -import org.apache.sis.test.TestConfiguration; import org.apache.sis.test.TestUtilities; import static org.apache.sis.test.Assertions.assertMapEquals; -import static org.apache.sis.test.TestUtilities.waitForGarbageCollection; /** @@ -36,7 +33,7 @@ import static org.apache.sis.test.TestUtilities.waitForGarbageCollection; * * @author Martin Desruisseaux (IRD, Geomatys) */ -public final class WeakValueHashMapTest extends TestCase { +public final class WeakValueHashMapTest extends TestCaseWithGC { /** * The size of the test sets to be created. */ @@ -160,7 +157,7 @@ public final class WeakValueHashMapTest extends TestCase { * could be caused by a heavy server load preventing GC to complete its work. If this * happen too often, we may turn off the "allow garbage collector dependent tests" flag. */ - if (TestConfiguration.allowGarbageCollectorDependentTests()) { + if (GC_DEPENDENT_TESTS_ENABLED) { waitForGarbageCollection(() -> weakMap.size() == strongMap.size()); assertMapEquals(strongMap, weakMap); /*