Using 'extracting' with InstanceOfAssertFactory will lead to an unexpected AssertionError if the extracted value is null

[kamil-sita]

Describe the bug
AssertJ allows extracting values from the original object using "extracting" method in order to test them in a more fluent way. AssertJ also allows using InstanceOfAssertFactory in "extracting" to specify which assertions should be used to test the extracted value.

However, there's an unexpected non-null check when using InstanceOfAssertFactory, which leads to an exception being thrown if the extracted value is null. This is not the case when not using InstanceOfAssertFactory, see the example below:

import org.assertj.core.api.InstanceOfAssertFactories;
import org.testng.annotations.Test;

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

public class Reproduced {

    @Test
    public void should_testNullCorrectly() {
        // given
        StringHolder holder = new StringHolder(null);

        // then
        assertThat(holder)
            .extracting(StringHolder::value)
            .isNull(); // this test will succeed
    }

    @Test
    public void should_testNullCorrectlyWithAssertFactory() {
        // given
        StringHolder holder = new StringHolder(null);

        // then
        assertThat(holder)
            .extracting(StringHolder::value, InstanceOfAssertFactories.STRING)
            .isNull(); // this test will fail
    }

    public record StringHolder(String value) { }
}

• assertj core version: tested with 3.16.1 and 3.24.2
• java version: 17.0.6
• test framework version: tested with TestNG 6.9.10 and JUnit 5.9.2

Test case reproducing the bug

import org.assertj.core.api.InstanceOfAssertFactories;
import org.testng.annotations.Test;

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

public class Reproduced {

    @Test
    public void should_testNullCorrectlyWithAssertFactory() {
        // given
        StringHolder holder = new StringHolder(null);

        // then
        assertThat(holder)
            .extracting(StringHolder::value, InstanceOfAssertFactories.STRING)
            .isNull(); // this test will fail
    }

    public record StringHolder(String value) { }
}

[scordio]

extracting(Function, InstanceOfAssertFactory) internally relies on asInstanceOf which performs a preliminary isInstanceOf check, the source of the AssertionError you are experiencing.

Is there a reason why you are using InstanceOfAssertFactories.STRING even though you already expect the value to be null?

On a side note, the Javadoc of extracting and asInstanceOf could be improved to cover such cases.

[kamil-sita]

Is there a reason why you are using InstanceOfAssertFactories.STRING even though you already expect the value to be null?

Not exactly, no. I have created helper methods for exporting properties of the objects I'm testing, like this:

public AbstractStringAssert<?> assertionForPropertyXyz() {
   // some complicated code I would prefer not to repeat tens of times
   
   return assertThat(abc)
       .extracting(Abc::getXyz, InstanceOfAssertFactories.STRING);
}

and have planned to use them in my code like so:

// test 1
   someObject.assertionForPropertyXyz().startsWith("lorem ipsum");
// test 2
   someObject.assertionForPropertyXyz().isNull();

I realize that it might be an uncommon approach, however I also think that having extracting methods behave differently here is very surprising. Personally I think that both should behave in more-or-less the same way.

[scordio]

It's a fair request 🙂 However, we need a more comprehensive analysis of what this change would mean, especially because we expose some more generic versions of extracting for third-party extensions.

We'll discuss it internally and get back to you.

[kamil-sita]

Thank you!

[kamil-sita]

Hi, I wonder if you had an occasion to discuss this issue internally?