Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

failNotSame message could emphasize reference equality #1626

Open
sabi0 opened this issue Sep 5, 2019 · 3 comments
Open

failNotSame message could emphasize reference equality #1626

sabi0 opened this issue Sep 5, 2019 · 3 comments

Comments

@sabi0
Copy link

sabi0 commented Sep 5, 2019

When calling assertSame for two different instances of identical objects the failure message will be quite confusing: "expected same:<[1, 2]> was not:<[1, 2]>".
See

fail(formatted + "expected same:<" + expected + "> was not:<" + actual

I think the clarity could be improved by adding identityHashCode to the message, like this:
"expected same:<[1, 2]@ba38e> was not:<[1, 2]@de440>"
Or maybe even with the class name:
"expected same:<java.util.List@ba38e [1, 2]> was not:<java.util.List@de440 [1, 2]>"

@panchenko
Copy link
Contributor

There are cases when identityHashCode is not needed, like enum or when it's already present in toString(), like default implementation in Object. It should not be added always for sure, a good question if it's worth determining cases like Collection or leave the implementation as is.

@sabi0
Copy link
Author

sabi0 commented Sep 6, 2019

Well, technically enum values could also produce identical toString() output. And though one wouldn't normally do that such assumption can not be made JUnit code.

Instead of targeting specific cases why not just address the essence of the issue - identical toString() outputs?
How about this?

String expectedString = String.valueOf(expected);
String actualString = String.valueOf(actual);
if (expectedString.equals(actualString)) {
    if (expected != null) {
        expectedString = expected.getClass().getName() + '@' + System.identityHashCode(expected) + ' ' + expectedString;
    }
    if (actual != null) {
        actualString = actual.getClass().getName() + '@' + System.identityHashCode(actual) + ' ' + actualString;
    }
}
fail(formatted + "expected same:<" + expectedString + "> was not:<" + actualString + '>');

Then even cases like assertSame(null, "null") would produce a clear output:

expected same:< null> was not:<java.lang.String@de440 null>

@marcphilipp
Copy link
Member

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

No branches or pull requests

3 participants