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