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

ComparingOnlyFieldsOfType only provided types for comparison #3207

Closed
timmisset opened this issue Oct 4, 2023 · 3 comments
Closed

ComparingOnlyFieldsOfType only provided types for comparison #3207

timmisset opened this issue Oct 4, 2023 · 3 comments
Labels
theme: recursive comparison An issue related to the recursive comparison
Milestone

Comments

@timmisset
Copy link

Describe the bug
Assuming that this is not intentionally designed I chose bug, but it might very well be a feature request to further tweak the recursiveComparison behavior:

When comparing a nested structure the types provided in the comparingOnlyFieldsOfTypes not only affect the comparison but also the collection of values to compare.

  • assertj core version: 3.24.2
  • java version: 17
  • test framework version: junit 5.9.2

Test case reproducing the bug

The following test case will pass. However, when I use (NestedClass.class, String.class) as arguments for the comparingOnlyFieldsOfTypes the assertion will fail.
As said, this is probably by design but would be really great is if there is an option that the collection of values to compare does not filter out the NestedClass so that "A" and "B" are still collected to be compared.

I have a nested tree where a specific class occurs at different locations and I only want to compare the identifier field. This would require me to put all branches in the comparison to make sure they are reached.

@Test
    void test() {
        TestClass a = new TestClass(new NestedClass("A"));
        TestClass b = new TestClass(new NestedClass("B"));

        assertThat(a).usingRecursiveComparison()
                .comparingOnlyFieldsOfTypes(String.class)
                .isEqualTo(b);
    }

    @RequiredArgsConstructor
    private class TestClass {
        private final NestedClass nestedClass;
    }

    @RequiredArgsConstructor
    private class NestedClass {
        private final String nestedClassField;
    }
@scordio scordio added the theme: recursive comparison An issue related to the recursive comparison label Oct 4, 2023
@joel-costigliola
Copy link
Member

Thanks for reporting this, we'll look into it shortly !

@joel-costigliola
Copy link
Member

This is a bug, we don't register the fields of types that don't match the compared types when these types could have fields whose type are in the compared types list.

@joel-costigliola joel-costigliola added this to the 3.25.0 milestone Oct 8, 2023
joel-costigliola added a commit that referenced this issue Oct 23, 2023
…rison only checked the fields of the first level of compared types ignoring nested fields of specified compared types.

fix #3207
@timmisset
Copy link
Author

timmisset commented Oct 24, 2023

As requested by @joel-costigliola

using version 3.24.2 the following assertion fails. Using 3.25.0-SNAPSHOT the assertion succeeds.

        BuildRecipe buildRecipe = TestBuildRecipeBuilder.getBuildRecipe();
        buildRecipe.getBuildItems().get(0).setBuildItemId("buildItemId");
        buildRecipe.getBuildItems().get(0).getBuildItemChildren().get(0).setBuildItemChildId("buildItemChildId");

        BuildRecipe buildRecipe2 = TestBuildRecipeBuilder.getBuildRecipe();
        buildRecipe2.getBuildItems().get(0).setBuildItemId("buildItemId");
        buildRecipe2.getBuildItems().get(0).getBuildItemChildren().get(0).setBuildItemChildId(null);

        // basic recursive comparison
        assertThat(buildRecipe2).usingRecursiveComparison().isNotEqualTo(buildRecipe);

        // compare only the types with the differences. This doesn't work in 3.24.2 but does work in 3.25.0
        assertThat(buildRecipe2).usingRecursiveComparison()
                .comparingOnlyFieldsOfTypes(AbstractBuildItem.class, BuildItemChild.class)
                .isNotEqualTo(buildRecipe);

        // compare ignoring the types with the differences. This works in both versions
        assertThat(buildRecipe2).usingRecursiveComparison()
                .ignoringFieldsOfTypes(AbstractBuildItem.class, BuildItemChild.class)
                .isEqualTo(buildRecipe);

Very nice!

I did notice one additional difference. The field name for 'setBuildItemId' is 'buildItemId' so I checked what would happen if I specified comparing only that fieldname:

        BuildRecipe buildRecipe = TestBuildRecipeBuilder.getBuildRecipe();
        buildRecipe.getBuildItems().get(0).setBuildItemId("buildItemId");
        buildRecipe.getBuildItems().get(0).getBuildItemChildren().get(0).setBuildItemChildId("buildItemChildId");

        BuildRecipe buildRecipe2 = TestBuildRecipeBuilder.getBuildRecipe();
        buildRecipe2.getBuildItems().get(0).setBuildItemId("buildItemId");
        buildRecipe2.getBuildItems().get(0).getBuildItemChildren().get(0).setBuildItemChildId(null);

        // basic recursive comparison
        assertThat(buildRecipe2).usingRecursiveComparison().isNotEqualTo(buildRecipe);

        // compare only the fields with the differences.
        // in 3.24.0 - Assertion error, objects are considered equal
        // in 3.25.0 - IllegalArgumentException "The following fields don't exist: {buildItemChildId}{buildItemId}"
        assertThat(buildRecipe2).usingRecursiveComparison()
                .comparingOnlyFields("buildItemId", "buildItemChildId")
                .isNotEqualTo(buildRecipe);

        // compare ignoring the fields with the differences, this results in the same assertion error for both 3.24.2 and 3.25.0-SNAPSHOT
        // in 3.24.2 - Assertion error:
        // field/property 'buildItems[0].buildItemChildren[0].buildItemChildId' differ:
        // - actual value  : null
        // - expected value: "buildItemChildId"
        // in 3.25.0 - Assertion error, same as in 3.24.0
        assertThat(buildRecipe2).usingRecursiveComparison()
                .ignoringFields("buildItemId", "buildItemChildId")
                .isEqualTo(buildRecipe);

@scordio scordio removed the type: bug label Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: recursive comparison An issue related to the recursive comparison
Projects
None yet
Development

No branches or pull requests

3 participants