On Thu, 5 Sep 2024 19:26:01 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> I agree when there's added value, but in this particular case I don't know >> what to add... >> Note that `convertBack()` is exercised in `reconstructedObjectMustBeEqual()`. > > - 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. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1746089993
