On Thu, 5 Sep 2024 19:57:35 GMT, Michael Strauß <[email protected]> wrote:
>> - does `convertBack` need a failing scenario?
>> - does `convertBack` accept null argument?
>> - since we are dealing with type erasure and possible quirks of `CssParser`,
>> would it make sense to test the case when a wrong type is being passed to
>> `convertBack`?
>>
>> Also, a more generic suggestion: in the absence of `@Nullable`, we probably
>> should specify whether an argument or return value may be null.
>
> I've added tests for a `null` argument, as well as for an unsupported type
> for the `BORDER_IMAGE_SOURCE` and `BACKGROUND_IMAGE` mappings.
>
> As for your third bullet point, I think you mean `convert` and not
> `convertBack`. You can only pass "wrong" things into the former, not the
> latter method. While more tests for `convert` would be good, they shouldn't
> be done as part of this PR since it is unrelated code.
you are probably right. I was thinking of a rather contrived case like
void wrong() {
SubPropertyConverter c = (SubPropertyConverter)converter;
c.convertBack(new Object());
}
my question about _documenting_ nullability still stands though.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1746121944
