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

HierarchyChecker does not use PrefabValues #48

Closed
GoogleCodeExporter opened this issue Mar 29, 2015 · 3 comments
Closed

HierarchyChecker does not use PrefabValues #48

GoogleCodeExporter opened this issue Mar 29, 2015 · 3 comments

Comments

@GoogleCodeExporter
Copy link

What steps will reproduce the problem?
Encountered the problem with abstract equals definition but it is more general.

abstract class SuperClass {
  @Override
  public abstract boolean equals(Object obj);
}

class SubClass extends SuperClass {
  private final int field;

  public SubClass(int field) {
    this.field = field;
  }

  @Override
  public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + field;
    return result;
  }

  @Override
  public boolean equals(Object obj) {
    if (this == obj)
      return true;
    if (obj == null)
      return false;
    if (getClass() != obj.getClass())
      return false;
    SubClass other = (SubClass) obj;
    return field == other.field;
  }
}

@Test
public void test() {
  EqualsVerifier
    .forExamples(new SubClass(0), new SubClass(1), new SubClass(2))
    .withPrefabValues(SuperClass.class, new SubClass(3), new SubClass(4))
    .usingGetClass().verify();
}


What is the expected output? What do you see instead?
I would expect a successful test run.
However, the test case successfully passes the AbstractDelegationChecker 
because it honors the PrefabValues in #safelyGetInstance. The HierarchyChecker 
does not use the PrefabValues in #checkSuperclass and I encountered an 
AbstractMethod error, since #equals is not implemented in the generated 
SuperClass instance.

What version of the product are you using? On what operating system?
Version 1.0.1

Please provide any additional information below.
The exception also occurs if instanceof is used instead of #getClass.
I guess the easiest solution would be to use the PrefabValues in the 
HierarchyChecker as well. A workaround would be to allow to bypass the specific 
tests.

Original issue reported on code.google.com by [email protected] on 1 Jul 2011 at 3:58

@GoogleCodeExporter
Copy link
Author

Using the prefabValues in the HierarchyChecker would be incorrect, because they 
are instances of the SubClass, so not only wouldn't we be comparing with an 
instance of the superclass anymore, we'd even be comparing with an instance of 
the class we were testing in the first place, rendering the check pointless.

Normally, for the purposes of EqualsVerifier, if the superclass is abstract, 
it's ok to create an anonymous subclass and substitute that for the actual 
superclass. But that doesn't work, because in your example the equals method is 
abstract as well. The upshot of this, is that there isn't an implementation of 
equals in the superclass (since it's abstract), so we don't have to check it. 
Although hashCode still should be checked. Also, what if it's the other way 
round? (Abstract hashCode, concrete equals.) I'll have to think about this one.

At the very least, there should be a more descriptive error message, so I'll go 
ahead and accept the issue. I hope to resolve it in the next release.

Original comment by [email protected] on 6 Jul 2011 at 9:07

  • Changed state: Accepted
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Would it make sense to omit the checkSuperclass completely if the superclass is 
abstract?
Then you won't have to distinguish all the different cases.

Original comment by [email protected] on 7 Jul 2011 at 12:15

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

That is indeed what I ended up doing. Also, I added a test that says that if 
equals is abstract, then hashCode should be too, and vice versa. Partially 
because I don't think it makes sense to make one of them abstract but not the 
other, and partially because that made it a lot easier to implement :).

The fix will be in the next release, which I just started working on.

Original comment by [email protected] on 7 Aug 2011 at 7:46

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

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

No branches or pull requests

1 participant