[Feature request] Add recursive comparison support to do the value resolution only by field (reflection)

[fegbers]

Hi,

Summary

If you have a getter that modifies the value or only returns a subset of the value, the current resolving strategy is error prone / unusable for recursive comparison tests (see example below). Perhaps it's possible to add the support to do the value resolution only by field via reflection? Perhabs a boolean configuration could override the current implementation? Currently, we use the recursive comparison for all our tests and love the fact that new properties of a class are always tested immediately. Thanks for the new kind of testing 👍🏼

Example

Note: This is only an example to illustrate the problem. The easy solution would be the usage of "clean" getters. But if you have a public API you couldn't just change the internal behaviour…

public class Example
{
    private Collection<String> value;

    public Example(Collection<String> value)
    {
        this.value = value;
    }

    public String getValue()
    {
        if (value == null)
        {
            return null;
        }
        return value.iterator().next();
    }
}

class ExampleTest
{
    @Test
    void testRecCom() // should fail
    {
        Assertions.assertThat(new Example(List.of("A", "B")))
                .usingRecursiveComparison()
                .ignoringAllOverriddenEquals()
                .withStrictTypeChecking()
                .isEqualTo(new Example(List.of("A")));
    }
}

Greetings!

[joel-costigliola]

The best way to go would be to specify in the configuration to only use fields, something like: comparingOnlyFields() and change the implementation to introspect fields only.

[btx0424]

Hello there. What about adding a bit logic to introspection/PropertyOrFieldSupport/getSimpleValue such that if comparingOnlyFields is set to true then we skip the property part and look straight into fields?

public Object getSimpleValue(String name, Object input) { 
    if (comparingOnlyFields){ // 
        // try as a field, then map key 
        ... 
    }else{ 
        // try as a property, then field, then map key 
        ... 
    }
    ...
}

I can implement this with tests if you like. Frankly speaking, we are a group of university students working on an assignment for the course Software Engineering, which requires us to contribute to the community by fixing a number of issues. We are willing to help with things we are capable of. You can let us know if there are other issues you want to fix but are too busy to do so.

Best regards.

[joel-costigliola]

@sustc11810424 thanks! we still need to decide the best way to go on this one. look for issues with label "good first issue", these are the ones that are not too complex to tackle.

[joel-costigliola]

@fegbers , we have finally got the time to address #2189 which is why I'm closing this issue.

I actually wrote a test using a ComparingFields introspection strategy that addresses this issue.

I'm planning to expose the ComparingFields introspection strategy implementation in AssertJ so that it's available out of the box for users like yourself (we'll likely do the same with ComparingProperties).

We need to discuss in the team to switch the default introspection strategy implementation to ComparingProperties, I'm closing this issue and might create another one to decide on the the default introspection strategy.

Feel free to comment on the issue if you feel it's not fully addressed and we'll be happy to reopen it.