assertj-automation icon indicating copy to clipboard operation
assertj-automation copied to clipboard

Migrate containsExactlyInAnyOrderEntriesOf() to containsExactly() for singleton maps

Open ash211 opened this issue 9 months ago • 1 comments

I had a similar complaint as https://github.com/palantir/assertj-automation/pull/360#issuecomment-1018726479, that rewriting .isEqualTo(Map.of( to .containsExactlyInAnyOrderEntriesOf(Map.of( is quite the mouthful and reduces readability.

Maybe for singleton maps (with one entry) we can rewrite to containsExactly(Map.entry( instead?

Example

    @Test
    void test() {
        Map<String, String> result = Map.of("key", "value");

        assertThat(result).isEqualTo(Map.of("key", "value")); // <-- (1) devs write this
        assertThat(result).containsExactlyInAnyOrderEntriesOf(Map.of("key", "value1")); // <-- (2) we rewrite to this
        assertThat(result).containsExactly(Map.entry("key", "value1")); // <-- (3) rewrite to this instead?
    }

I think for singleton maps this is equivalent, because singleton maps have only one order.

Here's what the failure messages look like for (2) and (3). Note that there is a small change in order expectation, from the (and in same order).

// from (2)
Expecting map:
  {"key"="value"}
to contain only:
  ["key"="value1"]
map entries not found:
  ["key"="value1"]
and map entries not expected:
  ["key"="value"]


// from (3)
Expecting actual:
  {"key"="value"}
to contain exactly (and in same order):  // <-- note
  ["key"="value1"]
but some elements were not found:
  ["key"="value1"]
and others were not expected:
  ["key"="value"]

cc @iamdanfox @carterkozak @pkoenig10

ash211 avatar Mar 06 '25 06:03 ash211

I don't love this idea. When reading code, I find consistency to be quite important in making it easy to understand 1) what the existing code is doing and 2) how to structure new code. As a reader I would be a bit confused why we sometimes use containsExactlyEntriesOf and sometimes use containsExactly. I think the fact that the latter is used for singletons would not be obvious.

And while the fact that requiring the entries to be ordered doesn't matter in practice for singleton maps, it's confusing to use an assertion that requires this (and states so in the assertion failure message) when that is not actually a requirement of the code being tested.

pkoenig10 avatar Mar 06 '25 09:03 pkoenig10