On Tue, 9 Jul 2024 17:11:22 GMT, Liam Miller-Cushon <[email protected]> wrote:
>> test/jdk/java/util/Map/MapFactories.java line 505:
>>
>>> 503:
>>> 504: @Test(expectedExceptions=UnsupportedOperationException.class)
>>> 505: public void immutableEntrySetAddAllDisallowed() {
>>
>> Looking back at MOAT, do you think we should add these into MOAT?
>> https://github.com/openjdk/jdk/blob/598af2e51464be089b64da4024e62865c2c6ec72/test/jdk/java/util/Collection/MOAT.java#L594-L619
>>
>> We just need to add calls to `testMapMutatorsAlwaysThrow` and
>> `testEmptyMapMutatorsAlwaysThrow` to check
>> `test(Empty)CollMutatorsAlwaysThrow(map.entrySet());`,
>> `test(Empty)CollMutatorsAlwaysThrow(map.keySet());`, and
>> `test(Empty)CollMutatorsAlwaysThrow(map.values());`
>
> `testCollMutatorsAlwaysThrow` expects a `Collection<Integer>` (not e.g. a
> `Collection<Entry<Integer, Integer>>`). MOAT could be refactored to handle
> that case. Do you think that's worth it, or have thoughts about what the
> cleanest way to do that would be?
There is `testImmutableCollection`/`testImmutableSet` that takes an arbitrary
nonexistent item for insertion/removal:
https://github.com/openjdk/jdk/blob/598af2e51464be089b64da4024e62865c2c6ec72/test/jdk/java/util/Collection/MOAT.java#L665
I think a refactor of a generic `testCollMutatorsAlwaysThrow(Collection<T> c, T
t)` and delegating the original Integer version to call
`testCollMutatorsAlwaysThrow(c, ABSENT_VALUE)` would not be invasive.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18522#discussion_r1670934363
