Property introspection not able to identify record accessors

assertj core version: 3.26.3
java version: 21
test framework version: junit-jupiter-api 5.9.3

Test case reproducing the bug

    private record DummyRecord(String name, int revision) {

        //This is the culprit, as the naming seems to affect assertJ's internal reflection-logic. Comparison works as expected when I rename this method to anything different. 
        public boolean isRevision() {
            return revision != 1;
        }
    }

    @Test public void recursiveComparison_javaRecords() {
        assertThat(new DummyRecord("Bob", 10)).usingRecursiveComparison().isEqualTo(new DummyRecord("Bob", 2)); //passes, but should fail
    }

@JaspreetSinghJassal

Thanks for reporting this @JaspreetSinghJassal, interesting issue !

Obviously, assertj changed the getter of isRevision to cause the assertion that should have been wrong to pass, and changing isRevision to getRevision does the same thing

We have observed a similar, probably related, issue that can be reproduced as follows:

package org.example;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

public class AssertJRecordBooleanTest {
  @Test
  void testAssertJWithRecord() {
    assertThat(new ReproducerRecord(true, "Some string"))
        .usingRecursiveComparison()
        .isNotEqualTo(new ReproducerRecord(true, "Some other string"));
  }

  record ReproducerRecord(
      Boolean isSomething,
      String something
  ) {
  }
}

I would expect the above test to pass, but AssertJ deems the two records equal and the test fails.

@dylanpiergies

Thanks for the additional example, @dylanpiergies! Unfortunately, this slipped under our radar.

We'll discuss within the team whether a fix can be shipped as part of 3.27.1, given that 3.27.0 was recently released.

@dylanpiergies

The problem is when we introspect the values to compare, we get the fields or properties, it turns out that isX is considered to be field/property X and thus is hides the real X property. When we get the value, we get the boolean one which in the test case are both true.

To avoid this issue, you can set the introspection strategy to only compare fields and ignore properties, with that the test fails as expected:

    assertThat(new DummyRecord("Bob", 10)).usingRecursiveComparison()
                                          .withIntrospectionStrategy(ComparingFields.COMPARING_FIELDS)
                                          .isEqualTo(new DummyRecord("Bob", 2));

fails with error

Expecting actual:
  DummyRecord[name=Bob, revision=10]
to be equal to:
  DummyRecord[name=Bob, revision=2]
when recursively comparing field by field, but found the following difference:

field/property 'revision' differ:
- actual value  : 10
- expected value: 2

@dylanpiergies your test behaves as expected if you use withIntrospectionStrategy(ComparingFields.COMPARING_FIELDS):

    assertThat(new ReproducerRecord(true, "Some string")).usingRecursiveComparison()
                                                         .withIntrospectionStrategy(ComparingFields.COMPARING_FIELDS)
                                                         .isNotEqualTo(new ReproducerRecord(true, "Some other string"));

I'm still wondering if there is something to fix here, as the default introspection strategy looks for fields and properties, if a field is shadowed by a property, it is normal that the property is "overriding" the field (in our implementation, properties have priority over fields since a property getter is usually public and a field private).

We probably should make ComparingFields the default introspection strategy, as its behavior is more obvious than the current default that tries fields and properties.

It might make sense to treat records slightly differently, taking into account the implicit relationship between their fields and accessors, but ultimately it's a design decision for the AssertJ developers.

Comparing only fields should suffice as a workaround for us - thanks for the advice.

It might make sense to treat records slightly differently, taking into account the implicit relationship between their fields and accessors

We might introduce specific test coverage to ComparingProperties to demonstrate that it doesn't work correctly with records today, and think how this could be improved.

Both examples reproducing the behavior show inconsistent naming, you end up with the same concept (ex: revision) that has different types, int when the revision concept is a field, and boolean when it is a property. It feels to me as a design smell, am I wrong ?

The root cause is that the property introspection cannot identify record accessors.

I raised #3710 to fix it.

Another example unrelated from the recursive comparison:

record Record(String field, boolean isField) {
}

@Test
void test() {
  Record actual = new Record("value", true);
  assertThat(actual).extracting("field").isEqualTo("value");
}

which, before the changes introduced in #3710, fails with:

org.opentest4j.AssertionFailedError: [Extracted: field] 
expected: "value"
 but was: true

Add `extracting(String)` record test case for property introspection

Navigation Menu

Repository navigation

Property introspection not able to identify record accessors #3539

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Property introspection not able to identify record accessors #3539

Description

Activity

joel-costigliola commented on Jul 15, 2024

lijunyzzZ commented on Jul 17, 2024

dylanpiergies commented on Dec 20, 2024

scordio commented on Dec 20, 2024

joel-costigliola commented on Dec 21, 2024

joel-costigliola commented on Dec 21, 2024

dylanpiergies commented on Dec 22, 2024

scordio commented on Dec 22, 2024

joel-costigliola commented on Dec 22, 2024

scordio commented on Dec 22, 2024

scordio commented on Dec 22, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions