fix(built_value_test): respect non comparable fields

[Leptopoda]

Fields annotated with compare: false were wrongly compared by the equalsBuilt matcher.

[davidmorgan]

Thanks! That makes sense, a couple of thoughts:

• I think implementing as you have by adding an == check means that if the check fails because of a field that is compared, it will still go on to report mismatches on fields that have compare: false? That sounds confusing. Unfortunately I can't think of a way to do this correctly without codegen/mirrors.
• It's not clear to me that we never want the old behavior, maybe we should have both? It could be added as a different equalsBuiltIgnoringIncomparable or equalsBuilt(..., onlyCheckComparable: true)? Then people would have to opt in to the new behaviour so I wouldn't worry so much about the first point.

What do you think? Thanks.

[Leptopoda]

Sorry for my late reply. I somehow forgot this PR.

What do you think?

TBH, I dislike both approaches, but I can see your point.
For now, I've opted for the extra parameter, as equalsBuiltIgnoringIncomparable would be really long and hard to read from a quick look.
I also thought about a way to change this behavior globally like BuiltValueMatcher.onlyCheckComparable = true but this would introduce some hidden behavior and would again need to be set for every test file so we wouldn't gain much.

[davidmorgan]

I'm still worried about the first point I raised: this doesn't actually give a correct failure message when

• onlyCheckComparable: true
• the value differs on both a comparable field and on a not-comparable field

then the compare with == will fail, it will go on to list the differences, but that list will also include the non-comparable fields that should not have been checked.

Given that the whole point of this helper is to improve the failure message, I wonder if people should just use expect(v1, v2) instead of expect(v1, equalsBuilt(v2)) when there are non-comparable fields that they care about.