NightOwl888 commented on code in PR #1128:
URL: https://github.com/apache/lucenenet/pull/1128#discussion_r1952841285
##########
src/Lucene.Net/Support/ConcurrentHashSet.cs:
##########
@@ -42,7 +43,8 @@ namespace Lucene.Net.Support
/// concurrently from multiple threads.
/// </remarks>
[DebuggerDisplay("Count = {Count}")]
- internal class ConcurrentHashSet<T> : ISet<T>, IReadOnlyCollection<T>,
ICollection<T>
+ internal class ConcurrentHashSet<T> : ISet<T>, IReadOnlyCollection<T>
+ where T : notnull
Review Comment:
Given the fact that some of the reference types that may be stored in Java
are Integer, Long, etc., it usually makes more sense to store value types in
.NET collections (or nullable value types).
I get that this is probably just to avoid adding tests for functionality
that we don't currently use, but if we are going to put an artificial
limitation here to save time, we should add a comment that this collection
technically supports nullable entries so we can remove this constraint if it is
ever needed. Ideally, we wouldn't put in here in the first place. Do note that
all `ISet<T>` and `IList<T>` collections in the BCL support nullable types for
`T`, so it is unusual to do this on a general collection type.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
Review Comment:
In .NET, please use `IsSupersetOf()` rather than `.containsAll()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -47,5 +171,278 @@ public void TestExceptWith()
Assert.IsTrue(hashSet.Contains(0));
Assert.IsTrue(hashSet.Contains(99));
}
+
+ /// <summary>
+ /// Create a set from Integers 1-5.
+ /// </summary>
+ /// <remarks>
+ /// In the Harmony tests, this returns a ConcurrentHashMap,
+ /// hence the name. Retaining the name, even though this is not a map,
+ /// for consistency with the original tests.
+ /// </remarks>
+ private static ConcurrentHashSet<object> Map5()
+ {
+ ConcurrentHashSet<object> map = new ConcurrentHashSet<object>();
+ assertTrue(map.IsEmpty);
+ map.Add(one);
+ map.Add(two);
+ map.Add(three);
+ map.Add(four);
+ map.Add(five);
+ assertFalse(map.IsEmpty);
+ assertEquals(5, map.Count);
+ return map;
+ }
+
+ /// <summary>
+ /// clear removes all items
+ /// </summary>
+ [Test]
+ public void TestClear()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ map.Clear();
+ assertEquals(map.size(), 0);
+ }
+
+ /// <summary>
+ /// Sets with same contents are equal
+ /// </summary>
+ [Test]
+ [Ignore("ConcurrentHashSet does not currently implement structural
Equals")]
+ public void TestEquals()
+ {
+ ConcurrentHashSet<object> map1 = Map5();
+ ConcurrentHashSet<object> map2 = Map5();
+ assertEquals(map1, map2);
+ assertEquals(map2, map1);
+ map1.Clear();
+ assertFalse(map1.Equals(map2));
+ assertFalse(map2.Equals(map1));
+ }
+
+ /// <summary>
+ /// contains returns true for contained value
+ /// </summary>
+ /// <remarks>
+ /// This was <c>testContainsKey</c> in the Harmony tests,
+ /// but we're using keys as values here.
+ /// </remarks>
+ [Test]
+ public void TestContains()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ assertTrue(map.contains(one));
+ assertFalse(map.contains(zero));
+ }
+
+ /// <summary>
+ /// enumeration returns an enumeration containing the correct
+ /// elements
+ /// </summary>
+ [Test]
+ public void TestEnumeration()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ using IEnumerator<object> e = map.GetEnumerator();
+ int count = 0;
+ while (e.MoveNext())
+ {
+ count++;
+ Assert.IsNotNull(e.Current); // LUCENENET specific - original
test did not have an assert here
+ }
+
+ assertEquals(5, count);
+ }
+
+ // LUCENENET - omitted testGet because it is not applicable to a set
+
+ /// <summary>
+ /// IsEmpty is true of empty map and false for non-empty
+ /// </summary>
+ [Test]
+ public void TestIsEmpty()
+ {
+ ConcurrentHashSet<object> empty = new ConcurrentHashSet<object>();
+ ConcurrentHashSet<object> map = Map5();
+ assertTrue(empty.IsEmpty);
+ assertFalse(map.IsEmpty);
+ }
+
+ // LUCENENET - omitted testKeys, testKeySet, testKeySetToArray,
testValuesToArray, testEntrySetToArray,
+ // testValues, testEntrySet
+
+ /// <summary>
+ /// UnionAll adds all elements from the given set
+ /// </summary>
+ /// <remarks>
+ /// This was adapted from testPutAll in the Harmony tests.
+ /// </remarks>
+ [Test]
+ public void TestUnionWith()
+ {
+ ConcurrentHashSet<object> empty = new ConcurrentHashSet<object>();
+ ConcurrentHashSet<object> map = Map5();
+ empty.UnionWith(map);
+ assertEquals(5, empty.size());
+ assertTrue(empty.Contains(one));
+ assertTrue(empty.Contains(two));
+ assertTrue(empty.Contains(three));
+ assertTrue(empty.Contains(four));
+ assertTrue(empty.Contains(five));
+ }
+
+ // LUCENENET - omitted testPutIfAbsent, testPutIfAbsent2, testReplace,
testReplace2, testReplaceValue, testReplaceValue2
+
+ /// <summary>
+ /// remove removes the correct value from the set
+ /// </summary>
+ [Test]
+ public void TestRemove()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ map.remove(five);
+ assertEquals(4, map.size());
+ assertFalse(map.Contains(five));
+ }
+
+ /// <summary>
+ /// remove(value) removes only if value present
+ /// </summary>
+ [Test]
+ public void TestRemove2()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ map.remove(five);
+ assertEquals(4, map.size());
+ assertFalse(map.Contains(five));
+ map.remove(zero); // LUCENENET specific - modified, zero is not in
the set
+ assertEquals(4, map.Count);
+ assertFalse(map.Contains(zero)); // LUCENENET specific - ensure
zero was not added
+ }
+
+ /// <summary>
+ /// size returns the correct values
+ /// </summary>
+ [Test]
+ public void TestCount()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ // ReSharper disable once CollectionNeverUpdated.Local - indeed,
that's what we're testing here
+ ConcurrentHashSet<object> empty = new ConcurrentHashSet<object>();
+ assertEquals(0, empty.Count);
+ assertEquals(5, map.Count);
+ }
+
+ // LUCENENET - testToString omitted, could use Collections.ToString
instead
+
+ /// <summary>
+ /// Cannot create with negative capacity
+ /// </summary>
+ [Test]
+ public void TestConstructor1() {
+ try
+ {
+ _ = new ConcurrentHashSet<object>(8, -1);
+ shouldThrow();
+ }
+ catch (ArgumentOutOfRangeException) // LUCENENET specific -
changed from IllegalArgumentException to ArgumentOutOfRangeException
+ {
+ }
+ }
+
+ /// <summary>
+ /// Cannot create with negative concurrency level
+ /// </summary>
+ [Test]
+ public void TestConstructor2()
+ {
+ try
+ {
+ _ = new ConcurrentHashSet<object>(-1, 100);
+ shouldThrow();
+ }
+ catch (ArgumentOutOfRangeException) // LUCENENET specific -
changed from IllegalArgumentException to ArgumentOutOfRangeException
+ {
+ }
+ }
+
+ // LUCENENET - testConstructor3 omitted, we don't have a
single-int-argument constructor
+ // LUCENENET - omitted testGet_NullPointerException,
testContainsKey_NullPointerException, testContainsValue_NullPointerException,
etc.
+ // LUCENENET - omitted testPut_NullPointerException, etc. due to
nullable reference type checking making them unnecessary
+ // LUCENENET - omitted testSerialization due to lack of serialization
support
+ // LUCENENET - omitted testSetValueWriteThrough as that is not
applicable to a set
+
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L1474-L1532
+ /// <summary>
+ /// Apache Harmony test for
java.util.Collections#synchronizedSet(java.util.Set), adapted for <see
cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ [Test]
+ public void TestSynchronizedSet()
+ {
+ HashSet<object> smallSet = new HashSet<object>();
+ for (int i = 0; i < 50; i++)
+ {
+ smallSet.add(objArray[i]);
+ }
+
+ const int numberOfLoops = 200;
+ ConcurrentHashSet<object> synchSet = new
ConcurrentHashSet<object>(smallSet); // was:
Collections.synchronizedSet(smallSet);
+ // Replacing the previous line with the line below should cause
the test
+ // to fail--the set below isn't synchronized
+ // ISet<object> synchSet = smallSet;
+
+ SynchCollectionChecker normalSynchChecker = new
SynchCollectionChecker(
+ synchSet, false, numberOfLoops);
+ SynchCollectionChecker offsetSynchChecker = new
SynchCollectionChecker(
+ synchSet, true, numberOfLoops);
+ ThreadJob normalThread = normalSynchChecker;
+ ThreadJob offsetThread = offsetSynchChecker;
+ normalThread.Start();
+ offsetThread.Start();
+ while ((normalSynchChecker.NumberOfChecks < numberOfLoops)
+ || (offsetSynchChecker.NumberOfChecks < numberOfLoops))
+ {
+ try
+ {
+ Thread.Sleep(10);
+ }
+ catch (Exception e) when (e.IsInterruptedException())
+ {
+ //Expected
+ }
+ }
+ assertTrue("Returned set corrupted by multiple thread access",
+ normalSynchChecker.Result
+ && offsetSynchChecker.Result);
+ try
+ {
+ normalThread.Join(5000);
+ offsetThread.Join(5000);
+ }
+ catch (Exception e) when (e.IsInterruptedException())
+ {
+ fail("join() interrupted");
+ }
+
+ ISet<object> mySet; // = new ConcurrentHashSet<object>(smallSet);
// was: Collections.synchronizedSet(smallSet);
+ // LUCENENET: our type does not allow nulls
+ // mySet.add(null);
+ // assertTrue("Trying to use nulls in list failed",
mySet.contains(null));
+
+ smallSet = new HashSet<object>();
+ for (int i = 0; i < 100; i++)
+ {
+ smallSet.add(objArray[i]);
+ }
+ // LUCENENET TODO: could port this class and all of the classes it
uses
+ // new Support_SetTest(new ConcurrentHashSet<object>(smallSet))
Review Comment:
Do note that some of these `Support_` tests from Harmony have been ported in
J2N.
https://github.com/NightOwl888/J2N/blob/main/tests/NUnit/J2N.Tests/Collections/
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
+ || col.containsAll(offsetCountingList)))
Review Comment:
In .NET, please use `IsSupersetOf()` rather than `.containsAll()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
+ || col.containsAll(offsetCountingList)))
+ result = false;
+ col.clear();
+ }
+ finally
+ {
+ UninterruptableMonitor.Exit(col);
+ }
+ if (offset)
+ col.addAll(offsetCountingList);
+ else
+ col.addAll(normalCountingList);
+ Interlocked.Increment(ref numberOfChecks); // was:
numberOfChecks++;
+ }
+ }
+
+ public SynchCollectionChecker(ISet<object> c, bool offset,
+ int totalChecks)
+ {
+ // The collection to test, whether to offset the filler values
by
+ // size or not, and the min number of iterations to run
+ totalToRun = totalChecks;
+ col = c;
+ int colSize = c.size();
+ normalCountingList = new List<object>(colSize);
+ offsetCountingList = new List<object>(colSize);
+ for (int counter = 0; counter < colSize; counter++)
+ normalCountingList.Add(counter);
+ for (int counter = 0; counter < colSize; counter++)
+ offsetCountingList.Add(counter + colSize);
+ col.clear();
+ if (offset)
+ col.addAll(offsetCountingList);
+ else
+ col.addAll(normalCountingList);
Review Comment:
In .NET, please use `UnionWith()` rather than `.addAll()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -47,5 +171,278 @@ public void TestExceptWith()
Assert.IsTrue(hashSet.Contains(0));
Assert.IsTrue(hashSet.Contains(99));
}
+
+ /// <summary>
+ /// Create a set from Integers 1-5.
+ /// </summary>
+ /// <remarks>
+ /// In the Harmony tests, this returns a ConcurrentHashMap,
+ /// hence the name. Retaining the name, even though this is not a map,
+ /// for consistency with the original tests.
+ /// </remarks>
+ private static ConcurrentHashSet<object> Map5()
+ {
+ ConcurrentHashSet<object> map = new ConcurrentHashSet<object>();
+ assertTrue(map.IsEmpty);
+ map.Add(one);
+ map.Add(two);
+ map.Add(three);
+ map.Add(four);
+ map.Add(five);
+ assertFalse(map.IsEmpty);
+ assertEquals(5, map.Count);
+ return map;
+ }
+
+ /// <summary>
+ /// clear removes all items
+ /// </summary>
+ [Test]
+ public void TestClear()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ map.Clear();
+ assertEquals(map.size(), 0);
+ }
+
+ /// <summary>
+ /// Sets with same contents are equal
+ /// </summary>
+ [Test]
+ [Ignore("ConcurrentHashSet does not currently implement structural
Equals")]
+ public void TestEquals()
+ {
+ ConcurrentHashSet<object> map1 = Map5();
+ ConcurrentHashSet<object> map2 = Map5();
+ assertEquals(map1, map2);
+ assertEquals(map2, map1);
+ map1.Clear();
+ assertFalse(map1.Equals(map2));
+ assertFalse(map2.Equals(map1));
+ }
+
+ /// <summary>
+ /// contains returns true for contained value
+ /// </summary>
+ /// <remarks>
+ /// This was <c>testContainsKey</c> in the Harmony tests,
+ /// but we're using keys as values here.
+ /// </remarks>
+ [Test]
+ public void TestContains()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ assertTrue(map.contains(one));
Review Comment:
Please use `Contains()` rather than `contains()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
+ || col.containsAll(offsetCountingList)))
+ result = false;
+ col.clear();
+ }
+ finally
+ {
+ UninterruptableMonitor.Exit(col);
+ }
+ if (offset)
+ col.addAll(offsetCountingList);
+ else
+ col.addAll(normalCountingList);
Review Comment:
In .NET, please use `UnionWith()` rather than `.addAll()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
+ || col.containsAll(offsetCountingList)))
+ result = false;
+ col.clear();
+ }
+ finally
+ {
+ UninterruptableMonitor.Exit(col);
+ }
+ if (offset)
+ col.addAll(offsetCountingList);
Review Comment:
In .NET, please use `UnionWith()` rather than `.addAll()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -47,5 +171,278 @@ public void TestExceptWith()
Assert.IsTrue(hashSet.Contains(0));
Assert.IsTrue(hashSet.Contains(99));
}
+
+ /// <summary>
+ /// Create a set from Integers 1-5.
+ /// </summary>
+ /// <remarks>
+ /// In the Harmony tests, this returns a ConcurrentHashMap,
+ /// hence the name. Retaining the name, even though this is not a map,
+ /// for consistency with the original tests.
+ /// </remarks>
+ private static ConcurrentHashSet<object> Map5()
+ {
+ ConcurrentHashSet<object> map = new ConcurrentHashSet<object>();
+ assertTrue(map.IsEmpty);
+ map.Add(one);
+ map.Add(two);
+ map.Add(three);
+ map.Add(four);
+ map.Add(five);
+ assertFalse(map.IsEmpty);
+ assertEquals(5, map.Count);
+ return map;
+ }
+
+ /// <summary>
+ /// clear removes all items
+ /// </summary>
+ [Test]
+ public void TestClear()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ map.Clear();
+ assertEquals(map.size(), 0);
+ }
+
+ /// <summary>
+ /// Sets with same contents are equal
+ /// </summary>
+ [Test]
+ [Ignore("ConcurrentHashSet does not currently implement structural
Equals")]
+ public void TestEquals()
+ {
+ ConcurrentHashSet<object> map1 = Map5();
+ ConcurrentHashSet<object> map2 = Map5();
+ assertEquals(map1, map2);
+ assertEquals(map2, map1);
+ map1.Clear();
+ assertFalse(map1.Equals(map2));
+ assertFalse(map2.Equals(map1));
+ }
+
+ /// <summary>
+ /// contains returns true for contained value
+ /// </summary>
+ /// <remarks>
+ /// This was <c>testContainsKey</c> in the Harmony tests,
+ /// but we're using keys as values here.
+ /// </remarks>
+ [Test]
+ public void TestContains()
+ {
+ ConcurrentHashSet<object> map = Map5();
+ assertTrue(map.contains(one));
+ assertFalse(map.contains(zero));
Review Comment:
Please use `Contains()` rather than `contains()`.
##########
src/Lucene.Net.Tests/Support/TestConcurrentHashSet.cs:
##########
@@ -23,8 +33,122 @@ namespace Lucene.Net
* limitations under the License.
*/
- public class TestConcurrentHashSet
+ /// <summary>
+ /// Tests for <see cref="ConcurrentHashSet{T}"/>.
+ /// </summary>
+ /// <remarks>
+ /// Some tests (those not marked with <see
cref="LuceneNetSpecificAttribute"/>)
+ /// are adapted from Apache Harmony's ConcurrentHashMapTest class. This
class
+ /// tests ConcurrentHashMap, which is a dictionary, but the key behavior is
+ /// similar to <see cref="ConcurrentHashSet{T}"/>.
+ /// </remarks>
+ public class TestConcurrentHashSet : JSR166TestCase
{
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L66-L76
+ private static readonly object[] objArray = LoadObjArray();
+
+ private static object[] LoadObjArray() // LUCENENET: avoid static
constructors
+ {
+ object[] objArray = new object[1000];
+ for (int i = 0; i < objArray.Length; i++)
+ {
+ objArray[i] = i;
+ }
+ return objArray;
+ }
+
+ /// <summary>
+ /// Used by <see cref="TestSynchronizedSet"/>
+ /// </summary>
+ /// <remarks>
+ /// Implements ThreadJob instead of Runnable, as that's what we have
access to here.
+ /// </remarks>
+ // Ported from
https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/CollectionsTest.java#L88-L159
+ public class SynchCollectionChecker : ThreadJob
+ {
+ private ISet<object> col; // LUCENENET: was Collection, but we
need to use ISet to access the containsAll method
+
+ // private int colSize; // LUCENENET: converted to local variable
+
+ private readonly int totalToRun;
+
+ private readonly bool offset;
+
+ private volatile int numberOfChecks /* = 0 */;
+
+ private bool result = true;
+
+ private readonly List<object> normalCountingList;
+
+ private readonly List<object> offsetCountingList;
+
+ public override void Run()
+ {
+ // ensure the list either contains the numbers from 0 to
size-1 or
+ // the numbers from size to 2*size -1
+ while (numberOfChecks < totalToRun)
+ {
+ UninterruptableMonitor.Enter(col);
+ try
+ {
+ if (!(col.Count == 0
+ || col.containsAll(normalCountingList)
+ || col.containsAll(offsetCountingList)))
+ result = false;
+ col.clear();
+ }
+ finally
+ {
+ UninterruptableMonitor.Exit(col);
+ }
+ if (offset)
+ col.addAll(offsetCountingList);
+ else
+ col.addAll(normalCountingList);
+ Interlocked.Increment(ref numberOfChecks); // was:
numberOfChecks++;
+ }
+ }
+
+ public SynchCollectionChecker(ISet<object> c, bool offset,
+ int totalChecks)
+ {
+ // The collection to test, whether to offset the filler values
by
+ // size or not, and the min number of iterations to run
+ totalToRun = totalChecks;
+ col = c;
+ int colSize = c.size();
+ normalCountingList = new List<object>(colSize);
+ offsetCountingList = new List<object>(colSize);
+ for (int counter = 0; counter < colSize; counter++)
+ normalCountingList.Add(counter);
+ for (int counter = 0; counter < colSize; counter++)
+ offsetCountingList.Add(counter + colSize);
+ col.clear();
+ if (offset)
+ col.addAll(offsetCountingList);
Review Comment:
In .NET, please use `UnionWith()` rather than `.addAll()`.
--
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: dev-unsubscr...@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
