From 10e245918f6e23756cdaa02380a687f0a1aa01ba Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Tue, 18 Jun 2002 02:51:12 +0000 Subject: [PATCH] Renamed supportsAdd and supportsRemove to isAddSupported and isRemoveSupported. Removed commented out tests for equals and hashCode (the collection contract does not specify a specific contract on equals and hashCode), and added a comment at the top of the class so that no one comes in later and re-adds the test cases. Removed dependence on HashBag. If HashBag has a bug, this may adversly affect the tests that are using it. Reimplemented without using HashBag. Modified tests that used makeFullCollection and makeCollection to use the fixture field "collection" along with resetEmpty() and resetFull(). This allowed for more calls to verify to ensure that calls to methods that should not modify a collection don't actually modify it. When removing using the iterator, an equivalent operation cannot really be performed on the confirmed collection for verification. After some investigation, changed the existing tests (tested for instances of Set, List and Bag) to one that is a bit more generic: A new flag (that test subclasses can override) which specifies whether the elements in the collection are distinguishable from each other (and such that it might matter which element is actually removed from the collection when the iterator's remove method is called). git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@130722 13f79535-47bb-0310-9956-ffa450edef68 --- .../commons/collections/TestCollection.java | 690 ++++++++++-------- 1 file changed, 394 insertions(+), 296 deletions(-) diff --git a/src/test/org/apache/commons/collections/TestCollection.java b/src/test/org/apache/commons/collections/TestCollection.java index 093e19ecd..452b39e13 100644 --- a/src/test/org/apache/commons/collections/TestCollection.java +++ b/src/test/org/apache/commons/collections/TestCollection.java @@ -1,7 +1,7 @@ /* - * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestCollection.java,v 1.6 2002/06/18 01:14:23 mas Exp $ - * $Revision: 1.6 $ - * $Date: 2002/06/18 01:14:23 $ + * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//collections/src/test/org/apache/commons/collections/Attic/TestCollection.java,v 1.7 2002/06/18 02:51:12 mas Exp $ + * $Revision: 1.7 $ + * $Date: 2002/06/18 02:51:12 $ * * ==================================================================== * @@ -80,7 +80,7 @@ import java.util.Set; *

* You should create a concrete subclass of this class to test any custom * {@link Collection} implementation. At minimum, you'll have to - * implement the {@link #makeCollection} method. You might want to + * implement the {@link #makeCollection()} method. You might want to * override some of the additional protected methods as well:

* * Element Population Methods

@@ -88,45 +88,46 @@ import java.util.Set; * Override these if your collection restricts what kind of elements are * allowed (for instance, if null is not permitted): *

* * Supported Operation Methods

* * Override these if your collection doesn't support certain operations: *

* * Fixture Methods

* - * For tests on modification operations (adds and removes), fixtures are - * used to verify that the the operation results in correct state for the - * collection. Basically, the modification is performed against your - * collection implementation, and an identical modification is performed - * against a confirmed collection implementation. A confirmed - * collection implementation is something like - * java.util.ArrayList, which is known to conform exactly to - * its collection interface's contract. After the modification takes - * place on both your collection implementation and the confirmed - * collection implementation, the two collections are compared to see if - * their state is identical. The comparison is usually much more - * involved than a simple equals test.

+ * Fixtures are used to verify that the the operation results in correct state + * for the collection. Basically, the operation is performed against your + * collection implementation, and an identical operation is performed against a + * confirmed collection implementation. A confirmed collection + * implementation is something like java.util.ArrayList, which is + * known to conform exactly to its collection interface's contract. After the + * operation takes place on both your collection implementation and the + * confirmed collection implementation, the two collections are compared to see + * if their state is identical. The comparison is usually much more involved + * than a simple equals test. This verification is used to ensure + * proper modifications are made along with ensuring that the collection does + * not change when read-only modifications are made.

* * The {@link #collection} field holds an instance of your collection * implementation; the {@link #confirmed} field holds an instance of the - * confirmed collection implementation. The {@link #resetEmpty} and - * {@link #resetFull} methods set these fields to empty or full collections, + * confirmed collection implementation. The {@link #resetEmpty()} and + * {@link #resetFull()} methods set these fields to empty or full collections, * so that tests can proceed from a known state.

* * After a modification operation to both {@link #collection} and - * {@link #confirmed}, the {@link #verify} method is invoked to compare - * the results. You may want to override {@link #verify} to perform + * {@link #confirmed}, the {@link #verify()} method is invoked to compare + * the results. You may want to override {@link #verify()} to perform * additional verifications. For instance, when testing the collection - * views of a map, {@link TestMap} overrides {@link #verify} to make sure - * the map is changed after the collection view is changed. + * views of a map, {@link TestMap} would override {@link #verify()} to make + * sure the map is changed after the collection view is changed. * * If you're extending this class directly, you will have to provide * implementations for the following: @@ -148,17 +149,29 @@ import java.util.Set; * If your {@link Collection} fails one of these tests by design, * you may still use this base set of cases. Simply override the * test case (method) your {@link Collection} fails. For instance, the - * {@link #testIteratorFailFast} method is provided since most collections + * {@link #testIteratorFailFast()} method is provided since most collections * have fail-fast iterators; however, that's not strictly required by the * collection contract, so you may want to override that method to do * nothing.

* * @author Rodney Waldhoff * @author Paul Jack - * @version $Id: TestCollection.java,v 1.6 2002/06/18 01:14:23 mas Exp $ + * @author Michael A. Smith + * @version $Id: TestCollection.java,v 1.7 2002/06/18 02:51:12 mas Exp $ */ public abstract class TestCollection extends TestObject { + // + // NOTE: + // + // Collection doesn't define any semantics for equals, and recommends you + // use reference-based default behavior of Object.equals. (And a test for + // that already exists in TestObject). Tests for equality of lists, sets + // and bags will have to be written in test subclasses. Thus, there is no + // tests on Collection.equals nor any for Collection.hashCode. + // + + // These fields are used by reset() and verify(), and any test // method that tests a modification. @@ -195,7 +208,7 @@ public abstract class TestCollection extends TestObject { /** - * Resets the {@link #collection} and {@link #confirmed} fields to empty + * Resets the {@link #collection} and {@link #confirmed} fields to full * collections. Invoke this method before performing a modification * test. */ @@ -204,21 +217,102 @@ public abstract class TestCollection extends TestObject { this.confirmed = makeConfirmedFullCollection(); } + /** + * Specifies whether equal elements in the collection are, in fact, + * distinguishable with information not readily available. That is, if a + * particular value is to be removed from the collection, then there is + * one and only one value that can be removed, even if there are other + * elements which are equal to it. + * + *

In most collection cases, elements are not distinguishable (equal is + * equal), thus this method defaults to return false. In some cases, + * however, they are. For example, the collection returned from the map's + * values() collection view are backed by the map, so while there may be + * two values that are equal, their associated keys are not. Since the + * keys are distinguishable, the values are. + * + *

This flag is used to skip some verifications for iterator.remove() + * where it is impossible to perform an equivalent modification on the + * confirmed collection because it is not possible to determine which + * value in the confirmed collection to actually remove. Tests that + * override the default (i.e. where equal elements are distinguishable), + * should provide additional tests on iterator.remove() to make sure the + * proper elements are removed when remove() is called on the iterator. + **/ + protected boolean areEqualElementsDistinguishable() { + return false; + } /** * Verifies that {@link #collection} and {@link #confirmed} have * identical state. */ protected void verify() { + int confirmedSize = confirmed.size(); assertEquals("Collection size should match confirmed collection's", - confirmed.size(), collection.size()); + confirmedSize, collection.size()); assertEquals("Collection isEmpty() result should match confirmed " + " collection's", confirmed.isEmpty(), collection.isEmpty()); - Bag bag1 = new HashBag(confirmed); - Bag bag2 = new HashBag(collection); - assertEquals("Collections should contain same elements with " + - " the same cardinality", bag1, bag2); + + // verify the collections are the same by attempting to match each + // object in the collection and confirmed collection. To account for + // duplicates and differing orders, each confirmed element is copied + // into an array and a flag is maintained for each element to determine + // whether it has been matched once and only once. If all elements in + // the confirmed collection are matched once and only once and there + // aren't any elements left to be matched in the collection, + // verification is a success. + + // copy each collection value into an array + Object[] confirmedValues = new Object[confirmedSize]; + + Iterator iter; + + iter = confirmed.iterator(); + int pos = 0; + while(iter.hasNext()) { + confirmedValues[pos++] = iter.next(); + } + + // allocate an array of boolean flags for tracking values that have + // been matched once and only once. + boolean[] matched = new boolean[confirmedSize]; + + // now iterate through the values of the collection and try to match + // the value with one in the confirmed array. + iter = collection.iterator(); + while(iter.hasNext()) { + Object o = iter.next(); + boolean match = false; + for(int i = 0; i < confirmedSize; i++) { + if(matched[i]) { + // skip values already matched + continue; + } + if(o == confirmedValues[i] || + (o != null && o.equals(confirmedValues[i]))) { + // values matched + matched[i] = true; + match = true; + break; + } + } + // no match found! + if(!match) { + fail("Collection should not contain a value that the " + + "confirmed collection does not have."); + } + } + + // make sure there aren't any unmatched values + for(int i = 0; i < confirmedSize; i++) { + if(!matched[i]) { + // the collection didn't match all the confirmed values + fail("Collection should contain all values that are in the " + + "confirmed collection"); + } + } } @@ -237,7 +331,7 @@ public abstract class TestCollection extends TestObject { * Returns a confirmed full collection. * For instance, an {@link java.util.ArrayList} for lists or a * {@link java.util.HashSet} for sets. The returned collection - * should contain the elements returned by {@link #getFullElements}. + * should contain the elements returned by {@link #getFullElements()}. * * @return a confirmed full collection */ @@ -246,39 +340,39 @@ public abstract class TestCollection extends TestObject { /** * Returns true if the collections produced by - * {@link #makeCollection} and {@link #makeFullCollection} + * {@link #makeCollection()} and {@link #makeFullCollection()} * support the add and addAll * operations.

* Default implementation returns true. Override if your collection * class does not support add or addAll. */ - protected boolean supportsAdd() { + protected boolean isAddSupported() { return true; } /** * Returns true if the collections produced by - * {@link #makeCollection} and {@link #makeFullCollection} + * {@link #makeCollection()} and {@link #makeFullCollection()} * support the remove, removeAll, * retainAll, clear and - * iterator().remove methods. + * iterator().remove() methods. * Default implementation returns true. Override if your collection * class does not support removal operations. */ - protected boolean supportsRemove() { + protected boolean isRemoveSupported() { return true; } /** * Returns an array of objects that are contained in a collection - * produced by {@link #makeFullCollection}. Every element in the + * produced by {@link #makeFullCollection()}. Every element in the * returned array must be an element in a full collection.

* The default implementation returns a heterogenous array of * objects with some duplicates and with the null element. * Override if you require specific testing elements. Note that if you - * override {@link #makeFullCollection}, you must override + * override {@link #makeFullCollection()}, you must override * this method to reflect the contents of a full collection. */ protected Object[] getFullElements() { @@ -292,7 +386,7 @@ public abstract class TestCollection extends TestObject { /** * Returns an array of elements that are not contained in a * full collection. Every element in the returned array must - * not exist in a collection returned by {@link #makeFullCollection}. + * not exist in a collection returned by {@link #makeFullCollection()}. * The default implementation returns a heterogenous array of elements * without null. Note that some of the tests add these elements * to an empty or full collection, so if your collection restricts @@ -312,9 +406,9 @@ public abstract class TestCollection extends TestObject { /** * Returns a full collection to be used for testing. The collection * returned by this method should contain every element returned by - * {@link #getFullElements}. The default implementation, in fact, + * {@link #getFullElements()}. The default implementation, in fact, * simply invokes addAll on an empty collection with - * the results of {@link #getFullElements}. Override this default + * the results of {@link #getFullElements()}. Override this default * if your collection doesn't support addAll. */ protected Collection makeFullCollection() { @@ -333,10 +427,10 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#add}. + * Tests {@link Collection#add(Object)}. */ public void testCollectionAdd() { - if (!supportsAdd()) return; + if (!isAddSupported()) return; Object[] elements = getFullElements(); for (int i = 0; i < elements.length; i++) { @@ -365,10 +459,10 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#addAll}. + * Tests {@link Collection#addAll(Collection)}. */ public void testCollectionAddAll() { - if (!supportsAdd()) return; + if (!isAddSupported()) return; resetEmpty(); Object[] elements = getFullElements(); @@ -394,7 +488,7 @@ public abstract class TestCollection extends TestObject { } assertEquals("Size should increase after addAll", size + elements.length, collection.size()); - + resetFull(); size = collection.size(); r = collection.addAll(Arrays.asList(getFullElements())); @@ -404,54 +498,68 @@ public abstract class TestCollection extends TestObject { assertTrue("Size should increase if addAll returns true", size < collection.size()); } else { - assertTrue("Size should not change if addAll returns false", - size == collection.size()); + assertEquals("Size should not change if addAll returns false", + size, collection.size()); } } /** - * If {@link #supportsAdd} returns false, tests that add operations + * If {@link #isAddSupported()} returns false, tests that add operations * raise UnsupportedOperationException. */ public void testUnsupportedAdd() { - if (supportsAdd()) return; + if (isAddSupported()) return; + resetEmpty(); try { - makeCollection().add(new Object()); + collection.add(new Object()); fail("Emtpy collection should not support add."); } catch (UnsupportedOperationException e) { // expected } + // make sure things didn't change even if the expected exception was + // thrown. + verify(); try { - makeCollection().addAll(Arrays.asList(getFullElements())); + collection.addAll(Arrays.asList(getFullElements())); fail("Emtpy collection should not support addAll."); } catch (UnsupportedOperationException e) { // expected } + // make sure things didn't change even if the expected exception was + // thrown. + verify(); + resetFull(); try { - makeFullCollection().add(new Object()); + collection.add(new Object()); fail("Full collection should not support add."); } catch (UnsupportedOperationException e) { // expected } - + // make sure things didn't change even if the expected exception was + // thrown. + verify(); + try { - makeFullCollection().addAll(Arrays.asList(getOtherElements())); + collection.addAll(Arrays.asList(getOtherElements())); fail("Full collection should not support addAll."); } catch (UnsupportedOperationException e) { // expected } + // make sure things didn't change even if the expected exception was + // thrown. + verify(); } /** - * Test {@link Collection#clear}. + * Test {@link Collection#clear()}. */ public void testCollectionClear() { - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; resetEmpty(); collection.clear(); // just to make sure it doesn't raise anything @@ -465,158 +573,116 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#contains}. + * Tests {@link Collection#contains(Object)}. */ public void testCollectionContains() { - Collection c = makeCollection(); - ArrayList elements = new ArrayList(); - elements.addAll(Arrays.asList(getFullElements())); - elements.addAll(Arrays.asList(getOtherElements())); - Iterator iter = elements.iterator(); - while (iter.hasNext()) { - assertTrue("Empty collection shouldn't contain element", - !c.contains(iter.next())); + Object[] elements; + + resetEmpty(); + elements = getFullElements(); + for(int i = 0; i < elements.length; i++) { + assertTrue("Empty collection shouldn'y contain element", + !collection.contains(elements[i])); } - - elements.clear(); - elements.addAll(Arrays.asList(getFullElements())); - c = makeFullCollection(); - iter = elements.iterator(); - while (iter.hasNext()) { - Object o = iter.next(); - assertTrue("Full collection should contain element " + o, - c.contains(o)); + // make sure calls to "contains" don't change anything + verify(); + + elements = getOtherElements(); + for(int i = 0; i < elements.length; i++) { + assertTrue("Empty collection shouldn'y contain element", + !collection.contains(elements[i])); } - - elements.clear(); - elements.addAll(Arrays.asList(getOtherElements())); - iter = elements.iterator(); - while (iter.hasNext()) { + // make sure calls to "contains" don't change anything + verify(); + + resetFull(); + elements = getFullElements(); + for(int i = 0; i < elements.length; i++) { + assertTrue("Full collection should contain element.", + collection.contains(elements[i])); + } + // make sure calls to "contains" don't change anything + verify(); + + resetFull(); + elements = getOtherElements(); + for(int i = 0; i < elements.length; i++) { assertTrue("Full collection shouldn't contain element", - !c.contains(iter.next())); + !collection.contains(elements[i])); } } /** - * Tests {@link Collection#containsAll}. + * Tests {@link Collection#containsAll(Collection)}. */ public void testCollectionContainsAll() { - Collection c = makeCollection(); + resetEmpty(); Collection col = new HashSet(); assertTrue("Every Collection should contain all elements of an " + - "empty Collection.",c.containsAll(col)); + "empty Collection.", collection.containsAll(col)); col.addAll(Arrays.asList(getOtherElements())); assertTrue("Empty Collection shouldn't contain all elements of " + - "a non-empty Collection.",!c.containsAll(col)); - - c = makeFullCollection(); + "a non-empty Collection.", !collection.containsAll(col)); + // make sure calls to "containsAll" don't change anything + verify(); + + resetFull(); assertTrue("Full collection shouldn't contain other elements", - !c.containsAll(col)); + !collection.containsAll(col)); col.clear(); col.addAll(Arrays.asList(getFullElements())); - assertTrue("Full collection should containAll full elements " + - c + " " + col, c.containsAll(col)); + assertTrue("Full collection should containAll full elements", + collection.containsAll(col)); + // make sure calls to "containsAll" don't change anything + verify(); + col = Arrays.asList(getFullElements()).subList(2, 5); assertTrue("Full collection should containAll partial full " + - "elements", c.containsAll(col)); + "elements", collection.containsAll(col)); assertTrue("Full collection should containAll itself", - c.containsAll(c)); + collection.containsAll(collection)); + + // make sure calls to "containsAll" don't change anything + verify(); col = new ArrayList(); col.addAll(Arrays.asList(getFullElements())); col.addAll(Arrays.asList(getFullElements())); assertTrue("Full collection should containAll duplicate full " + - "elements", c.containsAll(col)); + "elements", collection.containsAll(col)); + + // make sure calls to "containsAll" don't change anything + verify(); } - - /* --------------------------------- - - // Got rid of the equals() tests -- Collection doesn't define - // any semantics for equals, and recommends you use reference-based - // default behavior of Object.equals. (And a test for that already - // exists in TestObject). Tests for equality of lists, - // sets and bags will have to be written in test subclasses. - - public void testCollectionEqualsSelf() { - Collection c = makeCollection(); - assertEquals("A Collection should equal itself",c,c); - tryToAdd(c,"element1"); - assertEquals("A Collection should equal itself",c,c); - tryToAdd(c,"element1"); - tryToAdd(c,"element2"); - assertEquals("A Collection should equal itself",c,c); - } - - public void testCollectionEquals() { - Collection c1 = makeCollection(); - Collection c2 = makeCollection(); - assertEquals("Empty Collections are equal.",c1,c2); - - boolean added1_1 = tryToAdd(c1,"element1"); - if(added1_1) { - assertTrue("Empty Collection not equal to non-empty Collection.",!c2.equals(c1)); - assertTrue("Non-empty Collection not equal to empty Collection.",!c1.equals(c2)); - } - - boolean added1_2 = tryToAdd(c2,"element1"); - assertEquals("After duplicate adds, Collections should be equal.",c1,c2); - - boolean added2_1 = tryToAdd(c1,"element2"); - boolean added3_2 = tryToAdd(c2,"element3"); - if(added2_1 || added3_2) { - assertTrue("Should not be equal.",!c1.equals(c2)); - } - } - - public void testCollectionHashCodeEqualsSelfHashCode() { - Collection c = makeCollection(); - assertEquals("hashCode should be repeatable",c.hashCode(),c.hashCode()); - tryToAdd(c,"element1"); - assertEquals("after add, hashCode should be repeatable",c.hashCode(),c.hashCode()); - } - - public void testCollectionHashCodeEqualsContract() { - Collection c1 = makeCollection(); - if(c1.equals(c1)) { - assertEquals("[1] When two objects are equal, their hashCodes should be also.",c1.hashCode(),c1.hashCode()); - } - Collection c2 = makeCollection(); - if(c1.equals(c2)) { - assertEquals("[2] When two objects are equal, their hashCodes should be also.",c1.hashCode(),c2.hashCode()); - } - tryToAdd(c1,"element1"); - tryToAdd(c2,"element1"); - if(c1.equals(c2)) { - assertEquals("[3] When two objects are equal, their hashCodes should be also.",c1.hashCode(),c2.hashCode()); - } - } - - -------------------------- */ - - /** - * Tests {@link Collection#isEmpty}. + * Tests {@link Collection#isEmpty()}. */ public void testCollectionIsEmpty() { - Collection c = makeCollection(); - assertTrue("New Collection should be empty.",c.isEmpty()); + resetEmpty(); + assertEquals("New Collection should be empty.", + true, collection.isEmpty()); + // make sure calls to "isEmpty() don't change anything + verify(); - c = makeFullCollection(); - assertTrue("Full collection shouldn't be empty", !c.isEmpty()); + resetFull(); + assertEquals("Full collection shouldn't be empty", + false, collection.isEmpty()); + // make sure calls to "isEmpty() don't change anything + verify(); } /** - * Tests the read-only functionality of {@link Collection#iterator}. + * Tests the read-only functionality of {@link Collection#iterator()}. */ public void testCollectionIterator() { - Collection c = makeCollection(); - Iterator it1 = c.iterator(); - assertTrue("Iterator for empty Collection shouldn't have next.", - !it1.hasNext()); + resetEmpty(); + Iterator it1 = collection.iterator(); + assertEquals("Iterator for empty Collection shouldn't have next.", + false, it1.hasNext()); try { it1.next(); fail("Iterator at end of Collection should throw " + @@ -624,10 +690,12 @@ public abstract class TestCollection extends TestObject { } catch(NoSuchElementException e) { // expected } - - c = makeFullCollection(); - it1 = c.iterator(); - for (int i = 0; i < c.size(); i++) { + // make sure nothing has changed after non-modification + verify(); + + resetFull(); + it1 = collection.iterator(); + for (int i = 0; i < collection.size(); i++) { assertTrue("Iterator for full collection should haveNext", it1.hasNext()); it1.next(); @@ -635,11 +703,11 @@ public abstract class TestCollection extends TestObject { assertTrue("Iterator should be finished", !it1.hasNext()); ArrayList list = new ArrayList(); - it1 = c.iterator(); - for (int i = 0; i < c.size(); i++) { + it1 = collection.iterator(); + for (int i = 0; i < collection.size(); i++) { Object next = it1.next(); assertTrue("Collection should contain element returned by " + - "its iterator", c.contains(next)); + "its iterator", collection.contains(next)); list.add(next); } try { @@ -649,32 +717,16 @@ public abstract class TestCollection extends TestObject { } catch (NoSuchElementException e) { // expected } - - /* - Removed -- TestSet, TestBag and TestList should do this - Collection elements = Arrays.asList(getFullElements()); - if (c instanceof Set) { - assertTrue("Iterator should return unique elements", - new HashSet(list).equals(new HashSet(elements))); - } - if (c instanceof List) { - assertTrue("Iterator should return sequenced elements", - list.equals(elements)); - } - if (c instanceof Bag) { - assertTrue("Iterator should return duplicate elements", - new HashBag(list).equals(new HashBag(elements))); - } - - */ + // make sure nothing has changed after non-modification + verify(); } /** - * Tests removals from {@link Collection#iterator}. + * Tests removals from {@link Collection#iterator()}. */ public void testCollectionIteratorRemove() { - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; resetEmpty(); try { @@ -683,6 +735,7 @@ public abstract class TestCollection extends TestObject { } catch (IllegalStateException e) { // expected } + verify(); try { Iterator iter = collection.iterator(); @@ -693,33 +746,31 @@ public abstract class TestCollection extends TestObject { } catch (IllegalStateException e) { // expected } + verify(); resetFull(); int size = collection.size(); - HashBag bag = new HashBag(collection); Iterator iter = collection.iterator(); while (iter.hasNext()) { Object o = iter.next(); - bag.remove(o, 1); iter.remove(); - if ((collection instanceof Set) || (collection instanceof List) || - (collection instanceof Bag)) { - // Unfortunately, we can't get away with this for a straight - // collection that might have unordered duplicate elements, - // but it works for Bag, Set and List. + + // if the elements aren't distinguishable, we can just remove a + // matching element from the confirmed collection and verify + // contents are still the same. Otherwise, we don't have the + // ability to distinguish the elements and determine which to + // remove from the confirmed collection (in which case, we don't + // verify because we don't know how). + // + // see areEqualElementsDistinguishable() + if(!areEqualElementsDistinguishable()) { confirmed.remove(o); verify(); } + size--; - assertEquals("Collection should shrink after iterator.remove", - collection.size(), size); - if (bag.getCount(o) == 0) { - assertTrue("Collection shouldn't contain element after " + - "iterator.remove", !collection.contains(o)); - } else { - assertTrue("Collection should still contain element after " + - "iterator.remove", collection.contains(o)); - } + assertEquals("Collection should shrink by one after " + + "iterator.remove", size, collection.size()); } assertTrue("Collection should be empty after iterator purge", collection.isEmpty()); @@ -738,10 +789,10 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#remove}. + * Tests {@link Collection#remove(Object)}. */ public void testCollectionRemove() { - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; resetEmpty(); Object[] elements = getFullElements(); @@ -763,34 +814,33 @@ public abstract class TestCollection extends TestObject { int size = collection.size(); for (int i = 0; i < elements.length; i++) { resetFull(); - HashBag bag = new HashBag(collection); assertTrue("Collection should remove extant element", collection.remove(elements[i])); - if ((collection instanceof Set) || (collection instanceof List) || - (collection instanceof Bag)) { - // Can't do this for unordered straight collection... + + // if the elements aren't distinguishable, we can just remove a + // matching element from the confirmed collection and verify + // contents are still the same. Otherwise, we don't have the + // ability to distinguish the elements and determine which to + // remove from the confirmed collection (in which case, we don't + // verify because we don't know how). + // + // see areEqualElementsDistinguishable() + if(!areEqualElementsDistinguishable()) { confirmed.remove(elements[i]); verify(); } + assertEquals("Collection should shrink after remove", size - 1, collection.size()); - bag.remove(elements[i], 1); - if (bag.getCount(elements[i]) == 0) { - assertTrue("Collection shouldn't contain removed element", - !collection.contains(elements[i])); - } else { - assertTrue("Collection should still contain removed element", - collection.contains(elements[i])); - } } } /** - * Tests {@link Collection#removeAll}. + * Tests {@link Collection#removeAll(Collection)}. */ public void testCollectionRemoveAll() { - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; resetEmpty(); assertTrue("Emtpy collection removeAll should return false for " + @@ -802,7 +852,7 @@ public abstract class TestCollection extends TestObject { "nonempty input", !collection.removeAll(new ArrayList(collection))); verify(); - + resetFull(); assertTrue("Full collection removeAll should return false for " + "empty input", @@ -839,10 +889,10 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#retainAll}. + * Tests {@link Collection#retainAll(Collection)}. */ public void testCollectionRetainAll() { - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; resetEmpty(); List elements = Arrays.asList(getFullElements()); @@ -901,14 +951,15 @@ public abstract class TestCollection extends TestObject { /** - * Tests {@link Collection#size}. + * Tests {@link Collection#size()}. */ public void testCollectionSize() { - Collection c = makeCollection(); - assertEquals("Size of new Collection is 0.",0,c.size()); + resetEmpty(); + assertEquals("Size of new Collection is 0.", 0, collection.size()); - c = makeFullCollection(); - assertTrue("Size of full collection should be nonzero", c.size() != 0); + resetFull(); + assertTrue("Size of full collection should be greater than zero", + collection.size() > 0); } @@ -916,22 +967,45 @@ public abstract class TestCollection extends TestObject { * Tests {@link Collection#toArray()}. */ public void testCollectionToArray() { - Collection c = makeCollection(); + resetEmpty(); assertEquals("Empty Collection should return empty array for toArray", - 0, c.toArray().length); + 0, collection.toArray().length); - c = makeFullCollection(); - HashBag bag = new HashBag(c); - Object[] array = c.toArray(); + resetFull(); + Object[] array = collection.toArray(); assertEquals("Full collection toArray should be same size as " + - "collection", array.length, c.size()); + "collection", array.length, collection.size()); + Object[] confirmedArray = confirmed.toArray(); + assertEquals("length of array from confirmed collection should " + + "match the length of the collection's array", + confirmedArray.length, array.length); + boolean[] matched = new boolean[array.length]; + for (int i = 0; i < array.length; i++) { assertTrue("Collection should contain element in toArray", - c.contains(array[i])); - bag.remove(array[i], 1); + collection.contains(array[i])); + + boolean match = false; + // find a match in the confirmed array + for(int j = 0; j < array.length; j++) { + // skip already matched + if(matched[j]) continue; + if(array[i] == confirmedArray[j] || + (array[i] != null && array[i].equals(confirmedArray[j]))) { + matched[j] = true; + match = true; + break; + } + } + if(!match) { + fail("element " + i + " in returned array should be found " + + "in the confirmed collection's array"); + } + } + for(int i = 0; i < matched.length; i++) { + assertEquals("Collection should return all its elements in " + + "toArray", true, matched[i]); } - assertTrue("Collection should return all its elements in toArray", - bag.isEmpty()); } @@ -939,29 +1013,32 @@ public abstract class TestCollection extends TestObject { * Tests {@link Collection.toArray(Object[])}. */ public void testCollectionToArray2() { - Collection c = makeCollection(); + resetEmpty(); Object[] a = new Object[] { new Object(), null, null }; - Object[] array = c.toArray(a); + Object[] array = collection.toArray(a); assertEquals("Given array shouldn't shrink", array, a); assertEquals("Last element should be set to null", a[0], null); - - c = makeFullCollection(); + verify(); + + resetFull(); try { - array = c.toArray(new Void[0]); + array = collection.toArray(new Void[0]); fail("toArray(new Void[0]) should raise ArrayStore"); } catch (ArrayStoreException e) { // expected } + verify(); try { - array = c.toArray(null); + array = collection.toArray(null); fail("toArray(null) should raise NPE"); } catch (NullPointerException e) { // expected } + verify(); - array = c.toArray(new Object[0]); - a = c.toArray(); + array = collection.toArray(new Object[0]); + a = collection.toArray(); assertEquals("toArrays should be equal", Arrays.asList(array), Arrays.asList(a)); @@ -975,11 +1052,13 @@ public abstract class TestCollection extends TestObject { Class cl = (Class)classes.iterator().next(); a = (Object[])Array.newInstance(cl, 0); - array = c.toArray(a); + array = collection.toArray(a); assertEquals("toArray(Object[]) should return correct array type", a.getClass(), array.getClass()); assertEquals("type-specific toArrays should be equal", - Arrays.asList(array), Arrays.asList(c.toArray())); + Arrays.asList(array), + Arrays.asList(collection.toArray())); + verify(); } @@ -987,58 +1066,66 @@ public abstract class TestCollection extends TestObject { * Tests toString on a collection. */ public void testCollectionToString() { - Collection c = makeCollection(); - assertTrue("toString shouldn't return null", c.toString() != null); + resetEmpty(); + assertTrue("toString shouldn't return null", + collection.toString() != null); - c = makeFullCollection(); - assertTrue("toString shouldn't return null", c.toString() != null); + resetFull(); + assertTrue("toString shouldn't return null", + collection.toString() != null); } /** - * If supportsRemove() returns false, tests to see that remove + * If isRemoveSupported() returns false, tests to see that remove * operations raise an UnsupportedOperationException. */ public void testUnsupportedRemove() { - if (supportsRemove()) return; + if (isRemoveSupported()) return; + resetEmpty(); try { - makeCollection().clear(); + collection.clear(); fail("clear should raise UnsupportedOperationException"); } catch (UnsupportedOperationException e) { // expected } + verify(); try { - makeCollection().remove(null); + collection.remove(null); fail("remove should raise UnsupportedOperationException"); } catch (UnsupportedOperationException e) { // expected } + verify(); try { - makeCollection().removeAll(null); + collection.removeAll(null); fail("removeAll should raise UnsupportedOperationException"); } catch (UnsupportedOperationException e) { // expected } + verify(); try { - makeCollection().retainAll(null); + collection.retainAll(null); fail("removeAll should raise UnsupportedOperationException"); } catch (UnsupportedOperationException e) { // expected } + verify(); + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iterator = c.iterator(); + Iterator iterator = collection.iterator(); iterator.next(); iterator.remove(); fail("iterator.remove should raise UnsupportedOperationException"); } catch (UnsupportedOperationException e) { // expected } + verify(); } @@ -1047,34 +1134,39 @@ public abstract class TestCollection extends TestObject { * Tests that the collection's iterator is fail-fast. */ public void testCollectionIteratorFailFast() { - if (supportsAdd()) { + if (isAddSupported()) { + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.add(getOtherElements()[0]); + Iterator iter = collection.iterator(); + Object o = getOtherElements()[0]; + collection.add(o); + confirmed.add(o); iter.next(); fail("next after add should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { // expected } + verify(); + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.addAll(Arrays.asList(getOtherElements())); + Iterator iter = collection.iterator(); + collection.addAll(Arrays.asList(getOtherElements())); + confirmed.addAll(Arrays.asList(getOtherElements())); iter.next(); fail("next after addAll should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { // expected } + verify(); } - if (!supportsRemove()) return; + if (!isRemoveSupported()) return; + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.clear(); + Iterator iter = collection.iterator(); + collection.clear(); iter.next(); fail("next after clear should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { @@ -1083,30 +1175,32 @@ public abstract class TestCollection extends TestObject { // (also legal given spec) } + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.remove(getFullElements()[0]); + Iterator iter = collection.iterator(); + collection.remove(getFullElements()[0]); iter.next(); fail("next after remove should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { // expected } + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.removeAll(Arrays.asList(getFullElements()).subList(2,5)); + Iterator iter = collection.iterator(); + List sublist = Arrays.asList(getFullElements()).subList(2,5); + collection.removeAll(sublist); iter.next(); fail("next after removeAll should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { // expected } + resetFull(); try { - Collection c = makeFullCollection(); - Iterator iter = c.iterator(); - c.retainAll(Arrays.asList(getFullElements()).subList(2,5)); + Iterator iter = collection.iterator(); + List sublist = Arrays.asList(getFullElements()).subList(2,5); + collection.retainAll(sublist); iter.next(); fail("next after retainAll should raise ConcurrentModification"); } catch (ConcurrentModificationException e) { @@ -1122,6 +1216,10 @@ public abstract class TestCollection extends TestObject { * * Fails any Throwable except UnsupportedOperationException, * ClassCastException, or IllegalArgumentException is thrown. + * + * @deprecated explicitly check for allowed exceptions rather than using + * this method to assume any of UnsupportedOperationException, + * ClassCaseException, or IllegalArgumentException are allowed. */ protected boolean tryToAdd(Collection c,Object obj) { // FIXME: Delete this method after TestList is patched @@ -1144,7 +1242,7 @@ public abstract class TestCollection extends TestObject { /** * Returns a list of elements suitable for return by - * {@link getFullElements}. The array returned by this method + * {@link getFullElements()}. The array returned by this method * does not include null, but does include a variety of objects * of different types. Override getFullElements to return * the results of this method if your collection does not support @@ -1176,7 +1274,7 @@ public abstract class TestCollection extends TestObject { /** * Returns the default list of objects returned by - * {@link getOtherElements}. Includes many objects + * {@link getOtherElements()}. Includes many objects * of different types. */ public static Object[] getOtherNonNullElements() { @@ -1197,7 +1295,7 @@ public abstract class TestCollection extends TestObject { /** * Returns a list of string elements suitable for return by - * {@link getFullElements}. Override getFullElements to return + * {@link getFullElements()}. Override getFullElements to return * the results of this method if your collection does not support * heterogenous elements or the null element. */ @@ -1211,7 +1309,7 @@ public abstract class TestCollection extends TestObject { /** * Returns a list of string elements suitable for return by - * {@link getOtherElements}. Override getOtherElements to return + * {@link getOtherElements()}. Override getOtherElements to return * the results of this method if your collection does not support * heterogenous elements or the null element. */