Add unit tests for LimitedConcurrencyLevelTaskScheduler, #1110 (#1119)

* Add unit tests for LimitedConcurrencyLevelTaskScheduler, #1110

* Move TaskState into Extensions class and make private, #1110

Author: paulirwin

Directory.Build.targets

@@ -112,7 +112,6 @@
     <DefineConstants>$(DefineConstants);FEATURE_ASSEMBLY_GETCALLINGASSEMBLY</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_FILESTREAM_LOCK</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_TEXTWRITER_CLOSE</DefineConstants>
-    <DefineConstants>$(DefineConstants);FEATURE_THREADPOOL_UNSAFEQUEUEWORKITEM</DefineConstants>
     <DefineConstants>$(DefineConstants);FEATURE_TYPE_GETMETHOD__BINDINGFLAGS_PARAMS</DefineConstants>
 
   </PropertyGroup>

src/Lucene.Net.Tests/Support/Threading/JSR166TestCase.cs

@@ -1,5 +1,15 @@
-using Lucene.Net.Util;
+// From Apache Harmony tests:
+// https://github.com/apache/harmony/blob/trunk/classlib/modules/concurrent/src/test/java/JSR166TestCase.java
+using Lucene.Net.Util;
 using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Threading;
+using System.Threading.Tasks;
+using ThreadInterruptedException = System.Threading.ThreadInterruptedException;
+
+#nullable enable
 
 namespace Lucene.Net.Support.Threading
 {
@@ -20,82 +30,90 @@ namespace Lucene.Net.Support.Threading
      * limitations under the License.
      */
 
-    /**
-     * Base class for JSR166 Junit TCK tests.  Defines some constants,
-     * utility methods and classes, as well as a simple framework for
-     * helping to make sure that assertions failing in generated threads
-     * cause the associated test that generated them to itself fail (which
-     * JUnit does not otherwise arrange).  The rules for creating such
-     * tests are:
-     *
-     * <ol>
-     *
-     * <li> All assertions in code running in generated threads must use
-     * the forms {@link #threadFail}, {@link #threadAssertTrue}, {@link
-     * #threadAssertEquals}, or {@link #threadAssertNull}, (not
-     * <tt>fail</tt>, <tt>assertTrue</tt>, etc.) It is OK (but not
-     * particularly recommended) for other code to use these forms too.
-     * Only the most typically used JUnit assertion methods are defined
-     * this way, but enough to live with.</li>
-     *
-     * <li> If you override {@link #setUp} or {@link #tearDown}, make sure
-     * to invoke <tt>super.setUp</tt> and <tt>super.tearDown</tt> within
-     * them. These methods are used to clear and check for thread
-     * assertion failures.</li>
-     *
-     * <li>All delays and timeouts must use one of the constants <tt>
-     * SHORT_DELAY_MS</tt>, <tt> SMALL_DELAY_MS</tt>, <tt> MEDIUM_DELAY_MS</tt>,
-     * <tt> LONG_DELAY_MS</tt>. The idea here is that a SHORT is always
-     * discriminable from zero time, and always allows enough time for the
-     * small amounts of computation (creating a thread, calling a few
-     * methods, etc) needed to reach a timeout point. Similarly, a SMALL
-     * is always discriminable as larger than SHORT and smaller than
-     * MEDIUM.  And so on. These constants are set to conservative values,
-     * but even so, if there is ever any doubt, they can all be increased
-     * in one spot to rerun tests on slower platforms.</li>
-     *
-     * <li> All threads generated must be joined inside each test case
-     * method (or <tt>fail</tt> to do so) before returning from the
-     * method. The <tt> joinPool</tt> method can be used to do this when
-     * using Executors.</li>
-     *
-     * </ol>
-     *
-     * <p> <b>Other notes</b>
-     * <ul>
-     *
-     * <li> Usually, there is one testcase method per JSR166 method
-     * covering "normal" operation, and then as many exception-testing
-     * methods as there are exceptions the method can throw. Sometimes
-     * there are multiple tests per JSR166 method when the different
-     * "normal" behaviors differ significantly. And sometimes testcases
-     * cover multiple methods when they cannot be tested in
-     * isolation.</li>
-     *
-     * <li> The documentation style for testcases is to provide as javadoc
-     * a simple sentence or two describing the property that the testcase
-     * method purports to test. The javadocs do not say anything about how
-     * the property is tested. To find out, read the code.</li>
-     *
-     * <li> These tests are "conformance tests", and do not attempt to
-     * test throughput, latency, scalability or other performance factors
-     * (see the separate "jtreg" tests for a set intended to check these
-     * for the most central aspects of functionality.) So, most tests use
-     * the smallest sensible numbers of threads, collection sizes, etc
-     * needed to check basic conformance.</li>
-     *
-     * <li>The test classes currently do not declare inclusion in
-     * any particular package to simplify things for people integrating
-     * them in TCK test suites.</li>
-     *
-     * <li> As a convenience, the <tt>main</tt> of this class (JSR166TestCase)
-     * runs all JSR166 unit tests.</li>
-     *
-     * </ul>
-     */
+    /// <summary>
+    /// LUCENENET NOTE: This class has been adapted from the Apache Harmony
+    /// tests. The original javadoc is included below, and adapted where necessary.
+    /// <para />
+    ///
+    /// Base class for JSR166 Junit TCK tests.  Defines some constants,
+    /// utility methods and classes, as well as a simple framework for
+    /// helping to make sure that assertions failing in generated threads
+    /// cause the associated test that generated them to itself fail (which
+    /// JUnit does not otherwise arrange).  The rules for creating such
+    /// tests are:
+    ///
+    /// <list type="bullets">
+    ///
+    /// <item> All assertions in code running in generated threads must use
+    /// the forms <see cref="threadFail"/>, <see cref="threadAssertTrue"/>,
+    /// <see cref="threadAssertEquals(long,long)"/>, <see cref="threadAssertEquals(object,object)"/>
+    /// or <see cref="threadAssertNull"/>, (not
+    /// <c>fail</c>, <c>assertTrue</c>, etc.) It is OK (but not
+    /// particularly recommended) for other code to use these forms too.
+    /// Only the most typically used JUnit assertion methods are defined
+    /// this way, but enough to live with.</item>
+    ///
+    /// <item> If you override <see cref="SetUp"/> or <see cref="TearDown"/>, make sure
+    /// to invoke <c>base.SetUp</c> and <c>base.TearDown</c> within
+    /// them. These methods are used to clear and check for thread
+    /// assertion failures.</item>
+    ///
+    /// <item>All delays and timeouts must use one of the constants
+    /// <see cref="SHORT_DELAY_MS"/>, <see cref="SMALL_DELAY_MS"/>, <see cref="MEDIUM_DELAY_MS"/>,
+    /// <see cref="LONG_DELAY_MS"/>. The idea here is that a SHORT is always
+    /// discriminable from zero time, and always allows enough time for the
+    /// small amounts of computation (creating a thread, calling a few
+    /// methods, etc) needed to reach a timeout point. Similarly, a SMALL
+    /// is always discriminable as larger than SHORT and smaller than
+    /// MEDIUM.  And so on. These constants are set to conservative values,
+    /// but even so, if there is ever any doubt, they can all be increased
+    /// in one spot to rerun tests on slower platforms.</item>
+    ///
+    /// <item> All threads generated must be joined inside each test case
+    /// method (or <c>fail</c> to do so) before returning from the
+    /// method. The <see cref="joinPool"/> method can be used to do this when
+    /// using Executors.</item>
+    ///
+    /// </list>
+    ///
+    /// <para />
+    /// <b>Other notes</b>
+    /// <list type="bullet">
+    ///
+    /// <item> Usually, there is one testcase method per JSR166 method
+    /// covering "normal" operation, and then as many exception-testing
+    /// methods as there are exceptions the method can throw. Sometimes
+    /// there are multiple tests per JSR166 method when the different
+    /// "normal" behaviors differ significantly. And sometimes testcases
+    /// cover multiple methods when they cannot be tested in
+    /// isolation.</item>
+    ///
+    /// <item> The documentation style for testcases is to provide as javadoc
+    /// a simple sentence or two describing the property that the testcase
+    /// method purports to test. The javadocs do not say anything about how
+    /// the property is tested. To find out, read the code.</item>
+    ///
+    /// <item> These tests are "conformance tests", and do not attempt to
+    /// test throughput, latency, scalability or other performance factors
+    /// (see the separate "jtreg" tests for a set intended to check these
+    /// for the most central aspects of functionality.) So, most tests use
+    /// the smallest sensible numbers of threads, collection sizes, etc
+    /// needed to check basic conformance.</item>
+    ///
+    /// <item>The test classes currently do not declare inclusion in
+    /// any particular package to simplify things for people integrating
+    /// them in TCK test suites.</item>
+    ///
+    /// <!-- LUCENENET: not implemented
+    /// <item> As a convenience, the <c>main</c> of this class (JSR166TestCase)
+    /// runs all JSR166 unit tests.</item>
+    /// -->
+    ///
+    /// </list>
+    /// </summary>
     public class JSR166TestCase : LuceneTestCase
     {
-        ///**
+        // /**
         // * Runs all JSR166 unit tests using junit.textui.TestRunner
         // */
         //public static void main(String[] args)
@@ -255,7 +273,7 @@ public void threadAssertFalse(bool b)
          * If argument not null, set status to indicate current testcase
          * should fail
          */
-        public void threadAssertNull(object x)
+        public void threadAssertNull(object? x)
         {
             if (x != null)
             {
@@ -281,7 +299,7 @@ public void threadAssertEquals(long x, long y)
          * If arguments not equal, set status to indicate current testcase
          * should fail
          */
-        public void threadAssertEquals(object x, object y)
+        public void threadAssertEquals(object? x, object? y)
         {
             if (x != y && (x == null || !x.equals(y)))
             {
@@ -326,25 +344,25 @@ public void threadUnexpectedException(Exception ex)
             fail("Unexpected exception: " + ex);
         }
 
-        ///**
-        // * Wait out termination of a thread pool or fail doing so
-        // */
-        //public void joinPool(ExecutorService exec)
-        //{
-        //    try
-        //    {
-        //        exec.shutdown();
-        //        assertTrue(exec.awaitTermination(LONG_DELAY_MS, TimeUnit.MILLISECONDS));
-        //    }
-        //    catch (SecurityException ok)
-        //    {
-        //        // Allowed in case test doesn't have privs
-        //    }
-        //    catch (InterruptedException ie)
-        //    {
-        //        fail("Unexpected exception");
-        //    }
-        //}
+        /**
+         * Wait out termination of a thread pool or fail doing so
+         */
+        public void joinPool(TaskScheduler exec)
+        {
+            try
+            {
+                exec.Shutdown();
+                assertTrue(exec.AwaitTermination(TimeSpan.FromMilliseconds(LONG_DELAY_MS)));
+            }
+            // catch (SecurityException ok) // LUCENENET - not needed
+            // {
+            //     // Allowed in case test doesn't have privs
+            // }
+            catch (ThreadInterruptedException /*ie*/)
+            {
+                fail("Unexpected exception");
+            }
+        }
 
 
         /**
@@ -363,7 +381,141 @@ public void unexpectedException()
             fail("Unexpected exception");
         }
 
+        internal void ShortRunnable()
+        {
+            try
+            {
+                Thread.Sleep(SHORT_DELAY_MS);
+            }
+            catch (Exception e)
+            {
+                threadUnexpectedException(e);
+            }
+        }
+
+        internal void MediumRunnable()
+        {
+            try
+            {
+                Thread.Sleep(MEDIUM_DELAY_MS);
+            }
+            catch (Exception e)
+            {
+                threadUnexpectedException(e);
+            }
+        }
 
         // LUCENENET TODO: Complete port
     }
+
+    /// <summary>
+    /// LUCENENET specific - fake support for an API that feels like ThreadPoolExecutor.
+    /// </summary>
+    internal static class JSR166TestCaseExtensions
+    {
+        /// <summary>
+        /// LUCENENET specific - state to keep track of tasks.
+        /// <see cref="LimitedConcurrencyLevelTaskScheduler"/> removes tasks from the list when they complete,
+        /// so this class is needed to keep track of them.
+        /// </summary>
+        private class TaskState
+        {
+            private readonly TaskFactory _factory;
+            private readonly List<Task> _tasks = new();
+
+            public TaskState(TaskScheduler scheduler)
+            {
+                _factory = new TaskFactory(scheduler);
+            }
+
+            public void NewTask(Action action)
+            {
+                var task = _factory.StartNew(action);
+                _tasks.Add(task);
+            }
+
+            public int ActiveCount => _tasks.Count(t => t.Status == TaskStatus.Running);
+
+            public int CompletedCount => _tasks.Count(t => t.IsCompleted);
+
+            public int TaskCount => _tasks.Count;
+
+            public bool AllCompleted => _tasks.All(t => t.IsCompleted);
+
+            public bool JoinAll(TimeSpan timeout) => Task.WhenAll(_tasks).Wait(timeout);
+        }
+
+        private static readonly ConditionalWeakTable<TaskScheduler, TaskState> _taskFactories = new();
+
+        public static void Execute(this TaskScheduler scheduler, Action action)
+        {
+            if (!_taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                state = new TaskState(scheduler);
+                _taskFactories.Add(scheduler, state);
+            }
+
+            state.NewTask(action);
+        }
+
+        public static bool AwaitTermination(this TaskScheduler scheduler, TimeSpan timeout)
+        {
+            if (_taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                return state.JoinAll(timeout);
+            }
+
+            return true;
+        }
+
+        public static int GetActiveCount(this TaskScheduler scheduler)
+        {
+            if (_taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                // Approximate the number of running threads, which shouldn't exceed the concurrency level
+                return Math.Min(scheduler.MaximumConcurrencyLevel, state.ActiveCount);
+            }
+
+            return 0;
+        }
+
+        public static int GetCompletedTaskCount(this TaskScheduler scheduler)
+        {
+            if (_taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                return state.CompletedCount;
+            }
+
+            return 0;
+        }
+
+        public static int GetTaskCount(this TaskScheduler scheduler)
+        {
+            if (_taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                return state.TaskCount;
+            }
+
+            return 0;
+        }
+
+        public static void Shutdown(this TaskScheduler scheduler)
+        {
+            if (scheduler is LimitedConcurrencyLevelTaskScheduler lcl)
+            {
+                lcl.Shutdown();
+            }
+        }
+
+        public static bool IsTerminated(this TaskScheduler scheduler)
+        {
+            if (scheduler is LimitedConcurrencyLevelTaskScheduler lcl
+                && _taskFactories.TryGetValue(scheduler, out TaskState? state))
+            {
+                return lcl.IsShutdown && state.AllCompleted;
+            }
+
+            return false; // can't be shut down, so can't be terminated
+        }
+    }
 }