FasterXML / jackson-jr

Stand-alone data-binding module designed as a light-weight (and -featured) alternative to `jackson-databind`: will only deal with "Maps, Lists, Strings, wrappers and Java Beans" (jr-objects), or simple read-only trees (jr-stree)
Apache License 2.0
239 stars 31 forks source link

Allowing Records to bypass FieldNameGetterCheck #156

Closed Shounaks closed 4 months ago

Shounaks commented 4 months ago

154 : Letting Records bypass the check for USE_FIELD_MATCHING_GETTERS.

cowtowncoder commented 4 months ago

This would make sense, but unfortunately tests now fail: did you try running them?

Shounaks commented 4 months ago

yes, these are running on local, but I don't understand why they are failing the build... The changes doesn't look like they should break anything.

cowtowncoder commented 4 months ago

yes, these are running on local, but I don't understand why they are failing the build... The changes doesn't look like they should break anything.

Yes, agreed. Ok, I'll try to see: things do fail when run locally for me.

cowtowncoder commented 4 months ago

@Shounaks it is actually quite logical when looking at NPE... fixed.

Shounaks commented 4 months ago

@Shounaks it is actually quite logical when looking at NPE... fixed.

That's more strange, then why was the build failing in only one jdk version? 😆
image I don't understand this behavior...

cowtowncoder commented 4 months ago

That's easily explained: because Records are not available for JDK 8 or 11, I had to make test only available for JDK17 Maven profile. And since I did not realize profile as defined ONLY runs if building with JDK 17, JDK 21 did not run tests either. If this was not done, JDK 8 and JDK 11 test runs would have failed on compilation.

But I fixed that problem so 17 and 21 failed (8 and 11 passed because no tests were included).

You can check out jr-record-test/pom.xml to see how this works.

Shounaks commented 4 months ago

Ohh ok, so that was causing my Java17 setup to not fail in local because I was running all the tests together, rather than running this individual test, and for you it was failing on local most probably due to the same JDK version, hence the mismatch. Thanks for the explanation and guidance.

cowtowncoder commented 4 months ago

Well, actually, there was a problem with pom.xml when Groovy tests and Record test were combined -- Record test never even compiled. The problem was that the packaging type was defined as "pom", which then apparently builds nothing (or rather compiles nothing, and then can't run tests). I had to change that to "jar" (or "bundle" would work too).

And I figured out a way to prevent publishing of artifacts: we unfortunately have useless 2.17.0 and 2.17.1 pom releases but should not publish any more of those.

So basically project setup for tests was broken and no one (myself included) realized... since it did actually run the Groovy test.