Assertj-core: containsOnlyElementsOf method is misleading

[afayes]

Summary

containsOnlyElementsOf method is misleading

Example

        List<Integer> expectedValues = Arrays.asList(1, 2, 3);
        List<Integer> actualValues = Arrays.asList(1, 2);

        assertThat(actualValues).containsOnlyElementsOf(expectedValues);

You expect this to pass since actual values contains only elements of expectedValues values i.e.
actual values is a subset of expected values. But it fails:

java.lang.AssertionError:
Expecting:
<[1, 2]>
to contain only:
<[1, 2, 3]>
but could not find the following elements:
<[3]>

Java 8 specific ?

• YES : create Pull Request from the master branch

[joel-costigliola]

Although I agree it can be confusing, we can't change that because that would be break change for containsOnly and containsOnlyElementsOf that exist for a long time in AssertJ, moreover some people may find that it is normal for the assertion to fail because of the extra expected value.

In that case, you should use the more precise isSubsetOf assertion.

Maybe we should mention isSubsetOf in the Javadoc containsOnly and containsOnlyElementsOf, WDYT ?

[btrajkovski]

@joel-costigliola Is this up for grabs? I would like to contribute to this project, because I am using it on a daily basis :)
Currently searching for issues where I can help.

[joel-costigliola]

@btrajkovski good to hear! :)
The only thing to do here is to update the javadoc with a code example showing @afayes example failing as expected and pointing out isSubsetOf assertion.

[afayes]

Thanks for working on this guys!

[joel-costigliola]

Thanks @btrajkovski for improving the javadoc.

[TehBakker]

Totally agree it's misleading, I expected it to do what isSubsetOf is doing.

[joel-costigliola]

@TehBakker would renaming it to containsAllElementsOf make it obvious ?

[TehBakker]

Definitly yes

[razboo]

@joel-costigliola, got here because of the same confusion with the method name.
I think All is also a bit confusing (to me at least).
Why not call it containsExactlyElementsOf in the spirit of containsExactly?

[joel-costigliola]

because containsExactly checks the order and expects all duplicates to be listed.

I think I'll consider deprecating containsOnly and containsOnlyElementsOf in favor of containsAll and containsAllElementsOf if enough users find the former confusing.

[afayes]

containsAll and containsAllElementsOf is more clear and intuitive

[joel-costigliola]

@TehBakker any thoughts on my last comment ?

[scordio]

I think I'll consider deprecating containsOnly and containsOnlyElementsOf in favor of containsAll and containsAllElementsOf if enough users find the former confusing.

I just realized containsAll and containsAllElementsOf (plus containsAllEntriesOf for maps) already exist but they do not cover the "nothing else" requirement.

@joel-costigliola do you plan to change the behavior of the existing containsAll assertions or do you simply want to suggest the developers to use the existing containsAll assertions together with hasSameSizeAs?

[TehBakker]

yes seems good to me

[joel-costigliola]

@scordio I prefer to keep containsAll as it is not ambiguous, to check the "nothing else" requirement we should add containsExactlyInAnyOrder which I'm 99% sure is equivalent to containsAll + hasSameSizeAs.

[joel-costigliola]

Let's deprecate containsOnlyElementsOf in favor of containsAllElementsOf (I'm not sold to deprecate containsOnly as I think it is not as confusing as containsOnlyElementsOf).

[joel-costigliola]

BTW containsOnlyElementsOf already had an alias named: hasSameElementsAs