Assertj-core: NPE in DualValue.isPotentialCyclingValue while using recursive comparison

[epeee]

Summary

java.lang.NullPointerException
    at org.assertj.core.api.recursive.comparison.DualValue.isPotentialCyclingValue(DualValue.java:225)
    at org.assertj.core.api.recursive.comparison.DualValue.hasPotentialCyclingValues(DualValue.java:218)
    at org.assertj.core.api.recursive.comparison.RecursiveComparisonDifferenceCalculator$ComparisonState.pickDualValueToCompare(RecursiveComparisonDifferenceCalculator.java:100)
    at org.assertj.core.api.recursive.comparison.RecursiveComparisonDifferenceCalculator.determineDifferences(RecursiveComparisonDifferenceCalculator.java:200)
    at org.assertj.core.api.recursive.comparison.RecursiveComparisonDifferenceCalculator.determineDifferences(RecursiveComparisonDifferenceCalculator.java:188)
    at org.assertj.core.api.RecursiveComparisonAssert.determineDifferencesWith(RecursiveComparisonAssert.java:970)
    at org.assertj.core.api.RecursiveComparisonAssert.isEqualTo(RecursiveComparisonAssert.java:159)

It looks like we should check if java.lang.Class#getCanonicalName is null as well. According to the java doc this can be the case, see javadoc.

https://github.com/joel-costigliola/assertj-core/blob/a64f3fb4e942d85a671fedc98eca3d8e262772ff/src/main/java/org/assertj/core/api/recursive/comparison/DualValue.java#L221-L226

I will add a sample later on.

Example

<add example here>

[joel-costigliola]

good catch, my bad!

The fix should be easy as we want to detect non java.lang types we can simply return false if the canonical name is null.

I'll probably do a bugfix release for this

[epeee]

yep, a bugfix release would be great.

thx!

[joel-costigliola]

I'm curious if you experienced the error from a real test case.
I have added the test for local classes but not yet anonymous ones, it feels a bit of an edge case but that needs to be fixed anyway

[epeee]

Yes, it is from a real test.

a sample to reproduce it might be

  enum TEST {
    A1 {
      @Override
      boolean test() {
        return false;
      }
    };

    abstract boolean test();
  }

  class C {
    TEST t = TEST.A1;
  }

  @Test
  public void testB() {
    C actual = new C();
    C expected = new C();

    assertThat(actual).usingRecursiveComparison().isEqualTo(expected);
  }

I will have a look at the other test cases failing due to this one later on, most likely I can share another example later on today.

[joel-costigliola]

@epeee I have created https://github.com/joel-costigliola/assertj-core/pull/1868 to fix this issue and added you as a reviewer.
Feel free to push more commits if you find more test to add.

[epeee]

@joel-costigliola thx!
I was able to track all the failing test cases down to the samples you already added in the newly added test case.

[joel-costigliola]

3.16.1 released, should be available in a few hours in maven central

[epeee]

works like a charm.
thx for the fast turnaround!