Skip to content

Conversation

@wouterpolet
Copy link
Contributor

Check List:

Let me know if there is anything I missed.

I created tests that run on all case insensitive maps, but didn't add ones for normal ones. I wasn't sure if the existing test suite is already sufficient. I can still add that of course.

@scordio
Copy link
Member

scordio commented Feb 22, 2022

I created tests that run on all case insensitive maps, but didn't add ones for normal ones. I wasn't sure if the existing test suite is already sufficient. I can still add that of course.

I think the existing tests are not providing the needed coverage, especially for unmodifiable implementations.

The best would be having two parameterized tests like in Maps_assertContainsOnlyKeys_Test, so should_pass:

https://github.com/assertj/assertj-core/blob/8fa801095be09d75155bb6f159fb33f621512dcf/src/test/java/org/assertj/core/internal/maps/Maps_assertContainsOnlyKeys_Test.java#L86-L96

and should_fail:

https://github.com/assertj/assertj-core/blob/8fa801095be09d75155bb6f159fb33f621512dcf/src/test/java/org/assertj/core/internal/maps/Maps_assertContainsOnlyKeys_Test.java#L130-L144

@wouterpolet wouterpolet marked this pull request as draft February 26, 2022 15:09
* Catch the `UnsupportedOperationException` in `createEmptyMap` if the
clone cannot be clear
* Make `clone` method throw an `UnsupportedOperationException` if the
map is immutable
* Catch the `UnsupportedOperationException` in
`compareActualMapAndExpectedEntries` if the clone method throws it
@sarajuhosova sarajuhosova force-pushed the 2165-containsexactly-maps-not-using-equals branch from 74001de to 9824e79 Compare March 5, 2022 11:34
@wouterpolet wouterpolet marked this pull request as ready for review March 5, 2022 12:32
@wouterpolet
Copy link
Contributor Author

wouterpolet commented Mar 5, 2022

I added the tests and we fixed some errors that came from them 🙂

@scordio scordio added this to the 3.23.0 milestone May 7, 2022
@scordio scordio self-assigned this May 25, 2022
# Conflicts:
#	src/main/java/org/assertj/core/internal/Maps.java
@scordio scordio modified the milestones: 3.23.0, 3.24.0 May 25, 2022
@joel-costigliola
Copy link
Member

closing as it is addressed by #2623 which keeps @wouterpolet as the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

containsExactly does not work properly with maps not using equals to compare keys

4 participants