Unit tests for custom Map wrapper

A while ago I asked for a code review of my automatically keyed map and I have recently had the time to get around to writing some unit tests for it using JUnit4.

The test ensures that all of the "primary" functionality works, i.e. the things that could actually go wrong. The reason I don't test every method is that half of them forward their implementation to the map being wrapped.

Here is my current code (thanks to @Tunaki and @h.j.k. for their help in improving my old code):

AutoKeyedMap.java

`import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

/**
* A {@link Map} that is automatically keyed.
*
* @param the key type for the map
* @param the value type for the map
*/
public abstract class AutoKeyedMap implements Map {
private final Map map;
private boolean preventOverwrite;

/**
* Constructs a new automatically keyed map that wraps the provided map.
*
* @param map the map to wrap
* @param preventOverwrite prevent overwriting of existing keys
*/
public AutoKeyedMap(final Map map, final boolean preventOverwrite) {
this.map = map;
this.preventOverwrite = preventOverwrite;
}

/**
* Constructs a new automatically keyed map that wraps a {@link HashMap}.
*
* @param preventOverwrite prevent overwriting of existing keys
*/
public AutoKeyedMap(final boolean preventOverwrite) {
this(new HashMap(), preventOverwrite);
}

/**
* Constructs a new automatically keyed map that wraps the provided map.
*
* @param map the map to wrap
*/
public AutoKeyedMap(final Map map) {
this(map, false);
}

/**
* Constructs a new automatically keyed map that wraps a {@link HashMap}.
*/
public AutoKeyedMap() {
this(new HashMap());
}

/**
* Returns the key for the provided value.
*
*

* This method looks up or calculates a key for the provided value.

public final Set> entrySet() {
    return map.entrySet();
}

From the documentation:

Returns a Set view of the mappings contained in this map. The set is backed by the map, so changes to the map are reflected in the set, and vice-versa. If the map is modified while an iteration over the set is in progress (except through the iterator's own remove operation, or through the setValue operation on a map entry returned by the iterator) the results of the iteration are undefined. The set supports element removal, which removes the corresponding mapping from the map, via the Iterator.remove, Set.remove, removeAll, retainAll and clear operations. It does not support the add or addAll operations.

Basically,

Map map = new AutoKeyedMap<>(true);
map.put(null, 42);
for(Map.Entry entry = map.entrySet()){
    entry.setValue(-1);
}

your map doesn't guarantee that values will not be overridden.

/**
 * Puts all of the key/values pairs from the provided map into the wrapped map.
 *
 * @param m the map to retrieve the key/value pairs from
 *
 * @see #put(Object, Object)
 * @see Map#putAll(Map)
 */
public final void putAll(final Map m) {
    for (final Map.Entry e : m.entrySet()) {
        put(e.getKey(), e.getValue());
    }
}

This is a method that, whilst it's good that you implemented it, really needs more thought.

You only allow non-null keys. Thus this method can only work if you only insert a map which has zero entries OR 1 entry with a null key.

You cannot make two AutoKeyedMaps and merge them via putAll. Consider put(null, e.getValue()).

@Test(expected = UnsupportedOperationException.class)
public void testPutNonNullKey() {
    final Map autoKeyedMap = new HashCodeMap<>(new HashMap());

    autoKeyedMap.put(0, TEST_VALUE);
}

This test is violating TDD and just ticking off checkboxes and code coverage. If a feature is unsupported, why does it have tests to ensure that it is not accidentally supported? Throw IllegalArgumentException instead; You don't allow non-null keys, so passing in a non-null argument is illegal.

* 
 * If the key is not null then this method will throw an {@link UnsupportedOperationException}.
 *
 * This is due to the fact that the key is automatically derived by {@link #getKey(Object)} and should not be
 * provided manually.
 * 

You're violating Map's contract here: put's UnsupportedOperationException tends to mean "cannot put at all". From the documentation:

UnsupportedOperationException - if the put operation is not supported by this map

IllegalArgumentException - if some property of the specified key or value prevents it from being stored in this map

/**
 * A class that has a fake hash code (i.e. one that can be set at construction time).
 *
 * 
 * Note that this class breaks the {@code equals}/{@code hashCode} contract and should only be used for
 * testing.
 * 
 */
private static final class FakeHashCodeObject {
    private final int hashCode;

    FakeHashCodeObject(final int hashCode) {
        this.hashCode = hashCode;
    }

    public int hashCode() {
        return hashCode;
    }
}

Note that this class breaks the equals/hashCode contract and should only be used for testing.

No it doesn't.

HashCode docs:

Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer, provided no information used in equals comparisons on the object is modified. This integer need not remain consistent from one execution of an application to another execution of the same application.

Check. (internal variable is final)

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

Check (doesn't override equals, so the only way to get a hit here is to be the same object).

It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hash tables.

It fails this check, but that's not required.

As equals is not overridden, it can't violate that contract.

Do not lie in the comments, for it is worse than having no comments at all.

1: Is it a good idea to just test the methods that have the "complex" logic? Am I testing everything I should here?

Seems like you're doing fine, aside from the UnsupportedOperationException test.

2: How is my test documentation / commenting? Is it explicit enough (or even too explicit)? Does it tell you everything you need to know about the test?

I prefer to read test names; they're too short and don't tell me everything.

When I run my testing tool, I see the method names in the list as "test passed" o

StackExchange Code Review Q#133148, answer score: 4