On Fri, 30 May 2025 14:38:19 GMT, Jaikiran Pai <j...@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
