RFR: 8358015: Fix SequencedMap sequenced view method specifications

Stuart Marks smarks at openjdk.org
Fri May 30 23:13:50 UTC 2025


On Fri, 30 May 2025 14:38:19 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Overall this looks good to me. I just have a small question about the newly introduced text.
>> 
>> The copyright years on BasicMap.java and SequencedMap.java will need a update before integrating.
>
>> I just have a small question about the newly introduced text.
> 
> I just realized one other thing - the updated BasicMap.java fixes an issue in an existing test. Do we already have tests that might cover the new proposed semantics of these methods on the returned Set/Collection from the `SequencedMap.sequencedEntrySet/sequencedKeySet/sequenceValues`? Or should those be added?

@jaikiran

> Do we already have tests that might cover the new proposed semantics of these methods on the returned Set/Collection from the SequencedMap.sequencedEntrySet/sequencedKeySet/sequenceValues?

Good question. There are several existing tests in BasicMap.java; see checkKeySet, checkValues, and checkEntrySet. There is also a set of tests starting at testKeySetRemoves and following which test removals on several variations of the map views, and also other mutator methods. I don't see anything obvious missing, but that doesn't mean everything is actually tested! Probably at some point it would be good to do a comprehensive analysis of the collections tests and see if they can be unified. We should probably take a look at the test coverage report too. But this is all about functional testing, in the sense of: starting from this state and performing this operation, do we get the right results and side effects? (This is mostly _state verification_.)

The stuff in `@implSpec` that specifies *how* something is done might not be tested. In this case the implementation was done using method A and the `@implSpec` modified to require method B. The test made some unwarranted assumptions about the behavior of method A (which actually was unspecified) that in fact was not supported by method B; and so the test failed. Otherwise I probably wouldn't have noticed this. In general I don't think we don't do much _behavior_ verification in the JDK. If we do it's pretty ad hoc. In this case the question is, is there a test that verifies that a particular method is implemented by calling method B? I don't think so.

The terms _state verification_ and _behavior verification_ come from here:

https://martinfowler.com/articles/mocksArentStubs.html

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25515#issuecomment-2923697259


More information about the core-libs-dev mailing list