On Thu, 5 Sep 2024 19:18:09 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/BackgroundConverterTest.java
>> line 51:
>>
>>> 49: import static org.junit.jupiter.api.Assertions.*;
>>> 50:
>>> 51: public class BackgroundConverterTest {
>>
>> this is minor, but applicable to other tests:
>>
>> it would be nice to have a short basic description of the test, for human
>> consumption.
>>
>> Example:
>> Tests BackgroundConverter functions
>> - from url
>> - convert from image
>> - equality of reconstructed objects
>> (this is just an example)
>>
>> Also, should it also test `convertBack()` ?
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1522#discussion_r1746058328
