Update IndexProducerTest to test the behaviour

IndexProducers are tested for consistency between the indices output by
forEach and asIndexArray.

In addition the output methods can be tested that the indices are
ordered or distinct.

src/main/java/org/apache/commons/collections4/bloomfilter/IndexProducer.java

@@ -39,7 +39,7 @@ public interface IndexProducer {
      *
      * <p>Any exceptions thrown by the action are relayed to the caller.</p>
      *
-     * <p>Indices ordering is not guaranteed</p>
+     * <p>Indices ordering and uniqueness is not guaranteed.</p>
      *
      * @param predicate the action to be performed for each non-zero bit index.
      * @return {@code true} if all indexes return true from consumer, {@code false} otherwise.
@@ -103,6 +103,9 @@ public interface IndexProducer {
 
     /**
      * Return a copy of the IndexProducer data as an int array.
+     *
+     * <p>Indices ordering and uniqueness is not guaranteed.</p>
+     *
      * <p><em>
      * The default implementation of this method is slow.  It is recommended
      * that implementing classes reimplement this method.

src/test/java/org/apache/commons/collections4/bloomfilter/AbstractIndexProducerTest.java

@@ -16,34 +16,60 @@
  */
 package org.apache.commons.collections4.bloomfilter;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Arrays;
+import java.util.BitSet;
 import java.util.function.IntPredicate;
-
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.Test;
 
 public abstract class AbstractIndexProducerTest {
 
-    public static final IntPredicate TRUE_PREDICATE = new IntPredicate() {
+    private static final IntPredicate TRUE_PREDICATE = i -> true;
+    private static final IntPredicate FALSE_PREDICATE = i -> false;
 
-        @Override
-        public boolean test(int arg0) {
+    /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is ordered. */
+    protected static final int FOR_EACH_ORDERED = 0x1;
+    /** Flag to indicate the {@link IndexProducer#forEachIndex(IntPredicate)} is distinct. */
+    protected static final int FOR_EACH_DISTINCT = 0x2;
+    /** Flag to indicate the {@link IndexProducer#asIndexArray()} is ordered. */
+    protected static final int AS_ARRAY_ORDERED = 0x4;
+    /** Flag to indicate the {@link IndexProducer#asIndexArray()} is distinct. */
+    protected static final int AS_ARRAY_DISTINCT = 0x8;
+
+    /**
+     * An expandable list of int values.
+     */
+    private static class IntList {
+        private int size;
+        private int[] data = {0};
+
+        /**
+         * Adds the value to the list.
+         *
+         * @param value the value
+         * @return true if the list was modified
+         */
+        boolean add(int value) {
+            if (size == data.length) {
+                data = Arrays.copyOf(data, size << 1);
+            }
+            data[size++] = value;
             return true;
         }
-    };
 
-    public static final IntPredicate FALSE_PREDICATE = new IntPredicate() {
-
-        @Override
-        public boolean test(int arg0) {
-            return false;
+        /**
+         * Convert to an array.
+         *
+         * @return the array
+         */
+        int[] toArray() {
+            return Arrays.copyOf(data, size);
         }
-    };
+    }
 
     /**
      * Creates a producer with some data.
@@ -57,13 +83,22 @@ public abstract class AbstractIndexProducerTest {
      */
     protected abstract IndexProducer createEmptyProducer();
 
+    /**
+     * Gets the behaviour flags.
+     *
+     * <p>The flags indicate if the methods {@link IndexProducer#forEachIndex(IntPredicate)}
+     * and {@link IndexProducer#asIndexArray()} output sorted or distinct indices.
+     *
+     * @return the behaviour.
+     */
+    protected abstract int getBehaviour();
+
     @Test
     public final void testForEachIndex() {
-
         IndexProducer populated = createProducer();
         IndexProducer empty = createEmptyProducer();
-        assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false");
 
+        assertFalse(populated.forEachIndex(FALSE_PREDICATE), "non-empty should be false");
         assertTrue(empty.forEachIndex(FALSE_PREDICATE), "empty should be true");
 
         assertTrue(populated.forEachIndex(TRUE_PREDICATE), "non-empty should be true");
@@ -71,40 +106,77 @@ public abstract class AbstractIndexProducerTest {
     }
 
     @Test
-    public final void testAsIndexArray() {
-        int ary[] = createEmptyProducer().asIndexArray();
-        assertEquals(0, ary.length);
-
-        IndexProducer producer = createProducer();
-        List<Integer> lst = new ArrayList<Integer>();
-        for (int i : producer.asIndexArray()) {
-            lst.add(i);
-        }
-        assertTrue(producer.forEachIndex(new IntPredicate() {
-
-            @Override
-            public boolean test(int value) {
-                assertTrue(lst.remove(Integer.valueOf(value)),
-                        String.format("Instance of  %d was not found in lst", value));
-                return true;
-            }
+    public final void testEmptyProducer() {
+        IndexProducer empty = createEmptyProducer();
+        int ary[] = empty.asIndexArray();
+        Assertions.assertEquals(0, ary.length);
+        assertTrue(empty.forEachIndex(i -> {
+            throw new AssertionError("forEach predictate should not be called");
         }));
     }
 
+    /**
+     * Test the distinct indices output from the producer are consistent.
+     */
     @Test
-    public void testForIndexEarlyExit() {
+    public final void testConsistency() {
+        IndexProducer producer = createProducer();
+        BitSet bs1 = new BitSet();
+        BitSet bs2 = new BitSet();
+        Arrays.stream(producer.asIndexArray()).forEach(bs1::set);
+        producer.forEachIndex(i -> {
+            bs2.set(i);
+            return true;
+        });
+        Assertions.assertEquals(bs1, bs2);
+    }
+
+    @Test
+    public final void testBehaviourAsIndexArray() {
+        int flags = getBehaviour();
+        Assumptions.assumeTrue((flags & (AS_ARRAY_ORDERED | AS_ARRAY_DISTINCT)) != 0);
+        int[] actual = createProducer().asIndexArray();
+        if ((flags & AS_ARRAY_ORDERED) != 0) {
+            int[] expected = Arrays.stream(actual).sorted().toArray();
+            Assertions.assertArrayEquals(expected, actual);
+        }
+        if ((flags & AS_ARRAY_DISTINCT) != 0) {
+            long count = Arrays.stream(actual).distinct().count();
+            Assertions.assertEquals(count, actual.length);
+        }
+    }
+
+    @Test
+    public final void testBehaviourForEach() {
+        int flags = getBehaviour();
+        Assumptions.assumeTrue((flags & (FOR_EACH_ORDERED | FOR_EACH_DISTINCT)) != 0);
+        IntList list = new IntList();
+        createProducer().forEachIndex(list::add);
+        int[] actual = list.toArray();
+        if ((flags & FOR_EACH_ORDERED) != 0) {
+            int[] expected = Arrays.stream(actual).sorted().toArray();
+            Assertions.assertArrayEquals(expected, actual);
+        }
+        if ((flags & FOR_EACH_DISTINCT) != 0) {
+            long count = Arrays.stream(actual).distinct().count();
+            Assertions.assertEquals(count, actual.length);
+        }
+    }
+
+    @Test
+    public void testForEachIndexEarlyExit() {
         int[] passes = new int[1];
         assertFalse(createProducer().forEachIndex(i -> {
             passes[0]++;
             return false;
         }));
-        assertEquals(1, passes[0]);
+        Assertions.assertEquals(1, passes[0]);
 
         passes[0] = 0;
         assertTrue(createEmptyProducer().forEachIndex(i -> {
             passes[0]++;
             return false;
         }));
-        assertEquals(0, passes[0]);
+        Assertions.assertEquals(0, passes[0]);
     }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java

@@ -32,4 +32,10 @@ public class BitCountProducerFromArrayCountingBloomFilterTest extends AbstractBi
     protected BitCountProducer createEmptyProducer() {
         return new ArrayCountingBloomFilter(shape);
     }
+
+    @Override
+    protected int getBehaviour() {
+        // CountingBloomFilter based on an array will be distinct and ordered
+        return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromIndexProducerTest.java

@@ -20,13 +20,14 @@ import static org.junit.Assert.assertEquals;
 
 import java.util.HashMap;
 import java.util.Map;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProducerTest {
 
     @Override
     protected BitCountProducer createProducer() {
-        return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 1, 63, 64, 127, 128 }));
+        return BitCountProducer.from(IndexProducer.fromIndexArray(new int[] { 0, 63, 1, 1, 64, 127, 128 }));
     }
 
     @Override
@@ -34,7 +35,14 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu
         return BitCountProducer.from(IndexProducer.fromIndexArray(new int[0]));
     }
 
+    @Override
+    protected int getBehaviour() {
+        // The default method streams a BitSet so is distinct and ordered.
+        return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
+
     @Test
+    @Disabled("Current behaviour will return the same index twice, each with a count of 1")
     public final void testFromIndexProducer() {
 
         BitCountProducer producer = createProducer();
@@ -47,7 +55,7 @@ public class BitCountProducerFromIndexProducerTest extends AbstractBitCountProdu
 
         assertEquals(6, m.size());
         assertEquals(Integer.valueOf(1), m.get(0));
-        assertEquals(Integer.valueOf(1), m.get(1));
+        assertEquals(Integer.valueOf(2), m.get(1));
         assertEquals(Integer.valueOf(1), m.get(63));
         assertEquals(Integer.valueOf(1), m.get(64));
         assertEquals(Integer.valueOf(1), m.get(127));

src/test/java/org/apache/commons/collections4/bloomfilter/DefaultIndexProducerTest.java

@@ -45,6 +45,12 @@ public class DefaultIndexProducerTest extends AbstractIndexProducerTest {
         };
     }
 
+    @Override
+    protected int getBehaviour() {
+        // The default method streams a BitSet so is distinct and ordered.
+        return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
+
     /**
      * Generates an array of integers.
      * @param size the size of the array

src/test/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasherTest.java

@@ -35,6 +35,12 @@ public class EnhancedDoubleHasherTest extends AbstractHasherTest {
         return NullHasher.INSTANCE;
     }
 
+    @Override
+    protected int getBehaviour() {
+        // Allows duplicates and may be unordered
+        return 0;
+    }
+
     @Override
     protected int getHasherSize(Hasher hasher) {
         return 1;

src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java

@@ -41,14 +41,24 @@ public class HasherCollectionTest extends AbstractHasherTest {
         return new HasherCollection();
     }
 
+    @Override
+    protected int getBehaviour() {
+        // Allows duplicates and may be unordered
+        return 0;
+    }
+
     @Override
     protected int getHasherSize(Hasher hasher) {
         return ((HasherCollection) hasher).getHashers().size();
     }
 
     protected void nestedTest(HasherCollectionTest nestedTest) {
-        nestedTest.testAsIndexArray();
         nestedTest.testForEachIndex();
+        nestedTest.testEmptyProducer();
+        nestedTest.testConsistency();
+        nestedTest.testBehaviourAsIndexArray();
+        nestedTest.testBehaviourForEach();
+        nestedTest.testForEachIndexEarlyExit();
         nestedTest.testAdd();
     }
 

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java

@@ -32,4 +32,9 @@ public class IndexProducerFromArrayCountingBloomFilterTest extends AbstractIndex
     protected IndexProducer createEmptyProducer() {
         return new ArrayCountingBloomFilter(shape);
     }
+
+    @Override
+    protected int getBehaviour() {
+        return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromBitmapProducerTest.java

@@ -49,6 +49,12 @@ public class IndexProducerFromBitmapProducerTest extends AbstractIndexProducerTe
         return IndexProducer.fromBitMapProducer(producer);
     }
 
+    @Override
+    protected int getBehaviour() {
+        // Bit maps will be distinct. Conversion to indices should be ordered.
+        return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
+
     @Test
     public final void testFromBitMapProducerTest() {
         IndexProducer underTest = createProducer();

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherCollectionTest.java

@@ -27,4 +27,10 @@ public class IndexProducerFromHasherCollectionTest extends AbstractIndexProducer
     protected IndexProducer createEmptyProducer() {
         return new HasherCollection().indices(Shape.fromKM(17, 72));
     }
+
+    @Override
+    protected int getBehaviour() {
+        // HasherCollection allows duplicates and may be unordered
+        return 0;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromHasherTest.java

@@ -27,4 +27,10 @@ public class IndexProducerFromHasherTest extends AbstractIndexProducerTest {
     protected IndexProducer createEmptyProducer() {
         return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72));
     }
+
+    @Override
+    protected int getBehaviour() {
+        // Hasher allows duplicates and may be unordered
+        return 0;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromIntArrayTest.java

@@ -25,6 +25,12 @@ public class IndexProducerFromIntArrayTest extends AbstractIndexProducerTest {
 
     @Override
     protected IndexProducer createProducer() {
-        return IndexProducer.fromIndexArray(new int[] { 1, 2, 3, 4, 5 });
+        return IndexProducer.fromIndexArray(new int[] { 6, 8, 1, 2, 4, 4, 5 });
+    }
+
+    @Override
+    protected int getBehaviour() {
+        // Delegates to the default asIndexArray which is distinct and ordered
+        return AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
     }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSimpleBloomFilterTest.java

@@ -32,4 +32,10 @@ public class IndexProducerFromSimpleBloomFilterTest extends AbstractIndexProduce
     protected IndexProducer createEmptyProducer() {
         return new SimpleBloomFilter(shape);
     }
+
+    @Override
+    protected int getBehaviour() {
+        // BloomFilter based on a bit map array will be distinct and ordered
+        return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromSparseBloomFilterTest.java

@@ -33,4 +33,12 @@ public class IndexProducerFromSparseBloomFilterTest extends AbstractIndexProduce
     protected IndexProducer createEmptyProducer() {
         return new SparseBloomFilter(shape);
     }
+
+    @Override
+    protected int getBehaviour() {
+        // A sparse BloomFilter will be distinct but it may not be ordered.
+        // Currently the ordered behaviour is asserted as the implementation uses
+        // an ordered TreeSet. This may change in the future.
+        return FOR_EACH_DISTINCT | FOR_EACH_ORDERED | AS_ARRAY_DISTINCT | AS_ARRAY_ORDERED;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherCollectionTest.java

@@ -27,4 +27,13 @@ public class UniqueIndexProducerFromHasherCollectionTest extends AbstractIndexPr
     protected IndexProducer createEmptyProducer() {
         return new HasherCollection().uniqueIndices(Shape.fromKM(17, 72));
     }
+
+    @Override
+    protected int getBehaviour() {
+        // Note:
+        // Do not return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT.
+        // Despite this being a unique index test, the HasherCollection will return a unique
+        // index from each hasher. The result is there may still be duplicates.
+        return 0;
+    }
 }

src/test/java/org/apache/commons/collections4/bloomfilter/UniqueIndexProducerFromHasherTest.java

@@ -27,4 +27,10 @@ public class UniqueIndexProducerFromHasherTest extends AbstractIndexProducerTest
     protected IndexProducer createEmptyProducer() {
         return NullHasher.INSTANCE.indices(Shape.fromKM(17, 72));
     }
+
+    @Override
+    protected int getBehaviour() {
+        // Should be unique but may be unordered
+        return FOR_EACH_DISTINCT | AS_ARRAY_DISTINCT;
+    }
 }