Skip to content

Commit

Permalink
Simplify objenesis cache reset to improve error message with self-ref…
Browse files Browse the repository at this point in the history
…erring generics
  • Loading branch information
jqno committed Aug 21, 2024
1 parent 6d13518 commit 259b924
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 45 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- The error message in some edge cases involving complex generics and abstract classes is now improved. ([Issue 983](https://github.com/jqno/equalsverifier/issues/983))
- The line in the error message that shows the version of EqualsVerifier and the JDK, now also indicates whether EqualsVerifier runs on the classpath or the modulepath.

### Deprecated

- `withResetCaches()` was once needed for use in Quarkus, but caches are now reset automatically on every run.

## [3.16.1] - 2024-04-03

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache;
import nl.jqno.equalsverifier.internal.reflection.PackageScanner;
import nl.jqno.equalsverifier.internal.util.ListBuilders;
import nl.jqno.equalsverifier.internal.util.ObjenesisWrapper;
import nl.jqno.equalsverifier.internal.util.PrefabValuesApi;
import nl.jqno.equalsverifier.internal.util.Validations;

Expand Down Expand Up @@ -104,10 +103,13 @@ public ConfiguredEqualsVerifier withFieldnameToGetterConverter(
return this;
}

/** {@inheritDoc} */
/**
* {@inheritDoc}
* @deprecated No longer needed; this happens automatically.
*/
@Deprecated
@Override
public ConfiguredEqualsVerifier withResetCaches() {
ObjenesisWrapper.reset();
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ public interface EqualsVerifierApi<T> {
* that would normally be equal, to be unequal, because their ClassLoaders don't match.
*
* @return {@code this}, for easy method chaining.
* @deprecated No longer needed; this happens automatically.
*/
@Deprecated
EqualsVerifierApi<T> withResetCaches();
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import nl.jqno.equalsverifier.internal.util.ErrorMessage;
import nl.jqno.equalsverifier.internal.util.Formatter;
import nl.jqno.equalsverifier.internal.util.ListBuilders;
import nl.jqno.equalsverifier.internal.util.ObjenesisWrapper;
import nl.jqno.equalsverifier.internal.util.Validations;

/**
Expand Down Expand Up @@ -80,10 +79,13 @@ public MultipleTypeEqualsVerifierApi withFieldnameToGetterConverter(
return this;
}

/** {@inheritDoc} */
/**
* {@inheritDoc}
* @deprecated No longer needed; this happens automatically.
*/
@Deprecated
@Override
public MultipleTypeEqualsVerifierApi withResetCaches() {
ObjenesisWrapper.reset();
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,13 @@ public SingleTypeEqualsVerifierApi<T> withLombokCachedHashCode(T example) {
return this;
}

/** {@inheritDoc} */
/**
* {@inheritDoc}
* @deprecated No longer needed; this happens automatically.
*/
@Deprecated
@Override
public SingleTypeEqualsVerifierApi<T> withResetCaches() {
ObjenesisWrapper.reset();
return this;
}

Expand Down Expand Up @@ -402,6 +405,7 @@ private String buildErrorMessage(String description, boolean showUrl) {
}

private void performVerification() {
ObjenesisWrapper.reset();
if (type.isEnum() || type.isInterface()) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ private void wrappedChange(FieldChanger changer) throws IllegalAccessException {
try {
changer.change();
} catch (IllegalArgumentException e) {
if (e.getMessage().startsWith("Can not set")) {
String msg = e.getMessage();
if (msg.startsWith("Can not set") || msg.startsWith("Can not get")) {

Check warning on line 143 in equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 143 without causing a test to fail

negated conditional (0 tests run NegateConditionalsMutator)
throw new ReflectionException(
"Reflection error: perhaps a ClassLoader problem?\nTry re-running with #withResetCaches()",
"Reflection error: try adding a prefab value for field " +
field.getName() +
" of type " +
field.getType().getName(),
e
);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import java.util.concurrent.atomic.AtomicReferenceArray;
import java.util.function.Supplier;
import nl.jqno.equalsverifier.EqualsVerifier;
import nl.jqno.equalsverifier.Warning;
import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException;
import nl.jqno.equalsverifier.testhelpers.types.Point;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -93,6 +95,32 @@ public void succeed_whenClassHasTypeVariableThatExtendsSomethingThatSupersSometh
EqualsVerifier.forClass(TypeVariableExtendsWithSuperContainer.class).verify();
}

@Test
public void failGracefully_whenClassHasASelfReferenceGenericParameter() {
ExpectedException
.when(() -> EqualsVerifier.forClass(SelfReferringGenericType.class).verify())
.assertFailure()
.assertMessageContains(
"Reflection error",
"try adding a prefab value",
"field wrapped",
"of type " + SelfReferringGenericType.class.getName()
);
}

@Test
public void succeed_whenClassHasASelfReferenceGenericParameter_givenPrefabValues() {
EqualsVerifier
.forClass(SelfReferringGenericType.class)
.withPrefabValues(
SelfReferringGenericType.class,
new SelfReferringGenericType<>(1),
new SelfReferringGenericType<>(2)
)
.suppress(Warning.NONFINAL_FIELDS)
.verify();
}

static final class JavaGenericTypeContainer {

private final Optional<Point> optional;
Expand Down Expand Up @@ -704,4 +732,31 @@ public int hashCode() {
return defaultHashCode(this);
}
}

public static class SelfReferringGenericType<T extends SelfReferringGenericType<T>> {

private T wrapped;

// Everything below is boilerplate to be able to run the tests; the bit above this comment is what matters

private final int i;

public SelfReferringGenericType(int i) {
this.i = i;
}

@Override
public final boolean equals(Object obj) {
if (!(obj instanceof SelfReferringGenericType)) {
return false;
}
SelfReferringGenericType<?> other = (SelfReferringGenericType<?>) obj;
return i == other.i && Objects.equals(wrapped, other.wrapped);
}

@Override
public final int hashCode() {
return Objects.hash(i, wrapped);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,15 @@
import nl.jqno.equalsverifier.EqualsVerifier;
import nl.jqno.equalsverifier.internal.util.ObjenesisWrapper;
import nl.jqno.equalsverifier.testhelpers.packages.correct.A;
import nl.jqno.equalsverifier.testhelpers.packages.correct.B;
import org.junit.jupiter.api.Test;
import org.objenesis.Objenesis;

public class WithResetCachesTest {

@Test
public void single() {
public void resetObjenesisCacheOnEachRun() {
Objenesis original = ObjenesisWrapper.getObjenesis();
EqualsVerifier.forClass(A.class).withResetCaches();
Objenesis reset = ObjenesisWrapper.getObjenesis();
assertNotEquals(original, reset);
}

@Test
public void multiple() {
Objenesis original = ObjenesisWrapper.getObjenesis();
EqualsVerifier.forClasses(A.class, B.class).withResetCaches();
Objenesis reset = ObjenesisWrapper.getObjenesis();
assertNotEquals(original, reset);
}

@Test
public void configured() {
Objenesis original = ObjenesisWrapper.getObjenesis();
EqualsVerifier.configure().withResetCaches();
EqualsVerifier.forClass(A.class).verify();
Objenesis reset = ObjenesisWrapper.getObjenesis();
assertNotEquals(original, reset);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.reflect.Field;
import nl.jqno.equalsverifier.internal.exceptions.ReflectionException;
import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache;
import nl.jqno.equalsverifier.internal.prefabvalues.JavaApiPrefabValues;
import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues;
Expand Down Expand Up @@ -333,20 +331,6 @@ public void addPrefabArrayValues() {
assertEquals(RED_NEW_POINT, foo.points[0]);
}

@Test
public void shouldDetectClassloaderIssue() throws Exception {
// We're faking the problem by using two entirely different classes.
// In reality, this problem comes up with the same types, but loaded by different class loaders,
// which makes them effectively different types as well. This was hard to fake in a test.
Object foo = new ObjectContainer();
Field field = PrimitiveContainer.class.getField("field");
FieldModifier fm = FieldModifier.of(field, foo);

ReflectionException e = assertThrows(ReflectionException.class, () -> fm.set(new Object()));

assertTrue(e.getMessage().contains("perhaps a ClassLoader problem?"));
}

private void setField(Object object, String fieldName, Object value) {
getAccessorFor(object, fieldName).set(value);
}
Expand Down

0 comments on commit 259b924

Please sign in to comment.