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

java.lang.AssertionError: Significant fields: hashCode relies on serialVersionUID, but equals does not. These objects are equal, but probably shouldn't be: #159

Closed
xenoterracide opened this issue Oct 17, 2016 · 13 comments
Labels

Comments

@xenoterracide
Copy link

xenoterracide commented Oct 17, 2016

I can add more details later if necessary, I can't find a direct call to hashing serialVersionUID, I'm guessing this happens because of Serializable, by java itself.

private static final long serialVersionUID = 4492014110145565010L;

    /* super, and super of Super is object
        @Override
public int hashCode()
{
    return isNew() ? super.hashCode() : Objects.hashCode( getId() );
}
    */

@Override
public final int hashCode()
{
    return Objects.hash( super.hashCode(), id, external, display, integrationType );
}

@Test
public void testEquals() {
    EqualsVerifier.forClass( Integration.class )
        .withRedefinedSuperclass()
        .withOnlyTheseFields( "id", "external", "display", "integrationType" )
        .verify();
}

tried adding it to equals inside of the instanceof but that doesn't help

@jqno
Copy link
Owner

jqno commented Oct 18, 2016

Thanks for reporting this. I'm on vacation now, I will take a look when I get back.

@jqno
Copy link
Owner

jqno commented Nov 2, 2016

I can't be sure until I see your implementation of the equals method.

My first guess is that the super.hashCode() throws EqualsVerifier off. Every object instance will have a different value for super.hashCode(), even if the fields are equal to each other. In that case, your hashCode() doesn't follow the contract.

EqualsVerifier probably gets it mixed up with serialVersionUID because every time it changes the value for serialVersionUID, the hash code changes but equals doesn't. (Again, I'm guessing here, since I don't know what your equals() method looks like.) If you remove serialVersionUID from your class, you will probably get a slightly different error message, but one that still complains about your hash codes.

If that's not it, I need more information about your class. Ideally a complete class that reproduces the issue, that I can simply copy and paste into my IDE. Analysing these kinds of issues really takes up a lot of my free time.

@xenoterracide
Copy link
Author

I'll see what I can do to provide a more complete repro later, though equalsverifer changes serialVersionUID? that's static final. I'm willing to bet either way the whole thing is screwy, I put in a ticket to completely reimplement our super hashcode/equals it violates all kinds of things (esp symmetry), though I think when I reported this I thought I had fixed them for this class at least? but was unsure as to why/where it was even looking at serialVersionUID I'm also thinking this only happened in version 2.1...9? but didn't happen in version 1.7...5 (I know we're currently on 1.x, and that I tried upgrading to the most recent 2.x, and this was a new class)

but yeah, I'll see about providing more repro...

@xenoterracide
Copy link
Author

not so easy to reproduce in an isolated context... and I'd have to refix our super impls to fix all the symmetry and reflexivity errors to repro... some day...

@jqno
Copy link
Owner

jqno commented Nov 2, 2016

EqualsVerifier does indeed treat static final fields specially, because you're right, you can't change them. But it does iterate over all of them. Perhaps something happens in your code between the moment it inspects the field and the moment it decides to skip it because it's static and final.

Do you want me to keep this issue open while you try to isolate the problem?

@xenoterracide
Copy link
Author

oh, I don't know, I probably won't put any more effort into reproducing until such time that we reimplement equals, and I'm not sure when that'll be.

@jqno
Copy link
Owner

jqno commented Nov 4, 2016

OK, I'll close the issue then. If you decide to reproduce it after all, you're free to re-open it or file a new ticket.

@jqno jqno closed this as completed Nov 4, 2016
@eborgbjerg
Copy link

This can be reproduced by cloning my repo https://github.com/eborgbjerg/chessshell-api-1.git
and running the test SquareEqualsTest.

@jqno
Copy link
Owner

jqno commented Jun 29, 2020

@eborgbjerg Thanks! I took a look at your class. The underlying cause is that equals is inherited directly from Object, but hashCode does use the class's fields. If you move the serialVersionUID declaration to below the rankValue and fileValue fields, you will see a different kind of message: Significant fields: equals does not use rankValue.

I agree that the current message is quite misleading though. I'll see if I can do something about it.

@jqno jqno reopened this Jun 29, 2020
@jqno jqno added the accepted label Jun 29, 2020
@eborgbjerg
Copy link

Ok, I also tried adding
.withIgnoredFields("rankValue", "fileValue")
and commenting out serialVersionUID.

That gives the error
EqualsVerifier found a problem in class net.sourceforge.chessshell.domain.Square.
-> Significant fields: equals does not use rankValue.

Then, I tried adding
.suppress(Warning.ALL_FIELDS_SHOULD_BE_USED)

Then I get
EqualsVerifier found a problem in class net.sourceforge.chessshell.domain.Square.
-> Significant fields: equals relies on ALL_64, but hashCode does not.

It looks like the static fields on the Square class enter the picture, not sure whether this is correct though?

Apart from the above, I think I should change hashCode () to use a precalculated (final) value. But maybe the static fields would still get in the way of the unit test. Another option (and maybe the best) would be to rewrite the Square class as an enum.
But that would probably not make for the most interesting test of EqualsVerifier :-)

@jqno
Copy link
Owner

jqno commented Jun 30, 2020

The underlying problem is that the fields used to determine equals are not the same fields that determine the hashCode(): equals uses no fields, whereas hashCode() depends on rankValue and fileValue.

You can fix that by syncing those up (e.g. by removing your hashCode() method: does it really add value?), or by suppressing Warning.STRICT_HASHCODE.

@eborgbjerg
Copy link

I was able to get the unit test working (with the original version of Square) by using these options:
.suppress(Warning.ALL_FIELDS_SHOULD_BE_USED)
.suppress(Warning.STRICT_HASHCODE)

Thank you very much for your help, and for the tips for other improvements!

jqno added a commit that referenced this issue Sep 6, 2020
…s don't obscure issues in non-static fields
@jqno
Copy link
Owner

jqno commented Sep 8, 2020

I've just released version 3.4.3. In your situation, you will now get the Significant fields: equals does not use rankValue message instead of the one about serialVersionUID.

@jqno jqno closed this as completed Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants