Equals tester does not perform a check with the attributes set to nulls

[adamsiemion]

I have a class:

@AllArgsConstructor
@EqualsAndHashCode
public final class Entity {
    private String name;
}

and a test

assertPojoMethodsFor(Entity.class)
        .testing(CONSTRUCTOR, HASH_CODE, EQUALS)
        .areWellImplemented();

this test should result in 100% test coverage (I use Jacoco) of the Entity class, right? Unfortunately it does not. I used delombok to get the source code generated by lombok and from my research it seems that, in this case, equals() is never tested with name equal to null and that is the reason why for this class branch coverage is 64%. Can you please add new tests to EqualsTester?

A workaround for this issue is to add an additional test

assertPojoMethodsFor(Entity.class)
    .testing(CONSTRUCTOR, EQUALS, HASH_CODE)
    .create(Entity.class, new Object[] { null }, new Class<?>[] { String.class })
    .areWellImplemented();

[sta-szek]

Problem is with BestConstructorInstantiator.instantiate() method which tries to instantiate object with not null constructor parameters after AbstractMultiConstructorInstantiator.instantiateUsingUserParameters() returns null...

One of possible solutions for this problem is to create objects with default constructors parameters (0, null, false) in ObjectGenerator.createNewInstance(testedClass).
We can simply pass null parameters to constructor, but it can cause validation exceptions.
So better way is to set all field values to default after object generation.

Next problem to solve is to satisfy all conditions of lombok-generated equals method:

public boolean equals(final Object o) {
        if (o == this) {
            return true;
        } else if (!(o instanceof Entity)) {
            return false;
        } else {
            final Entity other = (Entity) o;
            final Object this$name = this.name;
            final Object other$name = other.name;
            if (this$name == null) {
                if (other$name != null) {
                    return false;
                }
            } else if (!this$name.equals(other$name)) {
                return false;
            }

            return true;
        }
    }

especially this one:

 if (this$name == null) {
                if (other$name != null) {
                    return false;
                }
            } else if (!this$name.equals(other$name)) {
                return false;
            }

This bunch of conditions needs three different objects, let's say:
Entity(null)
Entity(string)
Entity(otherString)

but pojo-tester generates only two of them:
Entity(null)
Entity(string)

So. We need to create one additional object for each field (recursively) with not null field value, which is very nontrivial.

The easiest way is to write additional test is AbstractTester.test(...) method which will create two objects with not null field values and then iterate over all of fields, of second one, and invoke AbstractFieldValueChanger.increaseValue(...) on it.

@SemionPar, are you willing to implement this? :)

[adamsiemion]

One of possible solutions for this problem is to create objects with default constructors parameters (0, null, false) in ObjectGenerator.createNewInstance(testedClass). We can simply pass null parameters to constructor, but it can cause validation exceptions.
So better way is to set all field values to default after object generation.

As you have written constructors can have validation checks so it might be tricky to use them, instead I think there is a simpler/better way - to use the Objenesis library, which does not use the existing constructors to instantiate the given class. Once a new instance is available the values of the fields can be changed using reflection.

I think this is the only way to guarantee 100% test coverage.

[sta-szek]

I was trying to minimize dependencies in pojo-tester but now, as you suggested, it could be better to use Objenesis.
Then classes within pl.pojo.tester.internal.instantiator package could be removed, which will simplify the project.

It would also simplify whole pl.pojo.tester.internal.field as all subclasses wouldn't have to implement AbstractFieldValueChanger<T>.public T increaseValue(final T value) method - nice idea for refactoring.

But know I think there is one way to make 100% coverage - we have to generate more distinct objects - let me (or you @adamsiemion? :) ) do this first when I have some more time.

[adamsiemion]

I did a little bit of research/tests and (as expected) it turns out not to be that trivial.

Replacing ObjectGenerator.createNewInstance() with:

new ObjenesisStd().newInstance(clazz);

Is not enough, because objenesis cannot instantiate primitive types, which is not a big problem, but unfortunately objenesis cannot also instantiate a lot of commonly used Java classes, such as e.g. BigDecimal. EqualsVerifier solved this by listing all such class (!) and providing two sample instances for each of these classes (JavaApiPrefabValues).

I have tried re-using the above class in ObjectGenerator:

public Object createNewInstance(final Class<?> clazz) {
    if (!clazz.isPrimitive() && !String.class.equals(clazz)) {
        Configuration<?> config = Configuration.of(clazz);
        final PrefabValues prefabValues = new PrefabValues();
        JavaApiPrefabValues.addTo(prefabValues);
        final Tuple<Object> tuple = prefabValues.giveTuple(config.getTypeTag());
        final Object black = tuple.getBlack();
        return black;
    }
    return Instantiable.forClass(clazz, constructorParameters).instantiate();
}

but unfortunately it does not work as some of the tests are failing, some are now hanging.

I will keep investigating the issue and share my findings.

@sta-szek are you ok with using EqualsVerifier classes in pojo-tester?

[sta-szek]

@adamsiemion thanks for investigating, but I prefer not to use EqualsVerifier in pojo tester as a dependency.
All we have to do is to generate more different objects as I mentioned above

Entity(null)
Entity(string)
Entity(otherString)
but pojo-tester generates only two of them:
Entity(null)
Entity(string)

And maybe perform more checks in EqualsTester's shouldNotEqualWithGivenFields method.

private void shouldNotEqualWithGivenFields(final ClassAndFieldPredicatePair baseClassAndFieldPredicatePair,
                                               final ClassAndFieldPredicatePair... classAndFieldPredicatePairs) {
        final List<Object> differentObjects = objectGenerator.generateDifferentObjects(baseClassAndFieldPredicatePair,
                                                                                       classAndFieldPredicatePairs);
        final Object firstObject = differentObjects.remove(0);
        differentObjects.forEach(assertIsNotEqualTo(firstObject));
    }

Maybe we should compare all objects, not only first one?