On Fri, 29 Apr 2022 00:06:32 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> test/jdk/java/util/IdentityHashMap/Basic.java line 257: >> >>> 255: checkEntries(map.entrySet(), entry(k1b, v1b), >>> 256: entry(k2, v2)); >>> 257: } >> >> Would an additional check `assertFalse(map.equals(map2));` be useful here >> (and other similar tests where we do "remove"). > > I don't know if you noticed that previous versions checked the map's contents > with `map.equals(map2)` and either asserted true or false depending on the > situation. I pulled most of those out when I added `checkEntries`. The reason > is that `checkEntries` and `checkElements` are supposed to check the exact > contents of the map or the collection, and they fail if there is a missing, > different, or extra entry or element. If that's true we don't need to test > `map.equals`. I don't think it calling `map.equals` adds any value or does > any checking that the `checkX` methods don't already do. > > Of course this relies on `checkEntries` and `checkElements` to do their jobs > properly, so we should make sure they do! > > And also we need to test that the `equals` method is doing its job as well. > There are a couple tests for it already, and they test at least the basic > cases. But it's possible I might have missed something. > > In any case, if we believe the `checkX` methods are sufficient, and if we > believe that the tests for `equals` are also sufficient, then I don't think > we need to add assertions about `equals` in any tests other than for `equals` > itself. Hello Stuart, Thank you for the explanation. > In any case, if we believe the checkX methods are sufficient, and if we > believe that the tests for equals are also sufficient, then I don't think we > need to add assertions about equals in any tests other than for equals itself. That makes sense. ------------- PR: https://git.openjdk.java.net/jdk/pull/8354
