Add tests for equals() and forEach() on keySet() and entrySet().
With the recent changes to make Entry objects mutable, should we add tests that make sure immutable maps and synchronized maps act correctly if someone tries to mutate with forEach?
With the recent changes to make
Entryobjects mutable, should we add tests that make sure immutable maps and synchronized maps act correctly if someone tries to mutate withforEach?
For sure! For context, I started working on #500 about 3 months ago, and I have lots of changes staged on a few branches, including lots of additional test coverage.
- https://github.com/eclipse/eclipse-collections/compare/master...motlin:eclipse-collections:unmodifiable-map
- https://github.com/eclipse/eclipse-collections/compare/master...motlin:eclipse-collections:unmodifiable-map-2
When I put stuff like this up for review, the reviews are too big to get anyone to read through. I'm finding them difficult to split up and land separately as well. Lots of the assertions that I pull up to superclasses expose real differences between the maps. None of them are earth shattering, but they slow me down.
- Whether Entries support mutation, which in turn affects whether default methods like Map.replace() throw
- Which exception is thrown when null is passed in
- Whether a method that may perform mutation like getIfAbsentPut always throws or only throws on mutation
- Other stuff that I'm forgetting now.
I'm confident that immutable maps will never mutate through Entries, but it's possible they throw different exceptions from different maps.
While I haven't pulled up tests of setValue() into the interface-based tests yet, there is already some coverage in the old-style tests. https://github.com/eclipse/eclipse-collections/blob/901272903c09c0c94c5fdbcfc3ffc91b1332ba9d/unit-tests/src/test/java/org/eclipse/collections/impl/map/mutable/UnmodifiableMutableMapTest.java#L439
This one has been open for a while and just adds test coverage. Any objections to landing it?