On Wed, 18 Sep 2024 22:40:20 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This PR converts all tests in `modules/javafx.graphics` to use JUnit5. >> >> ## Details >> >> Trivial changes resolved by first four commits: >> - Import changes to use `org.junit.jupiter` package instead of `org.junit` >> or `junit.framework` and all changes that follow it (ex. `@Before` -> >> `@BeforeEach`) >> - Message Strings in `assert*` calls moved to the end to follow 5's argument >> order >> - Some simple Parametrized tests (aka. those which only did a >> straightforward assignment of test parameters) adjusted to use JUnit5's >> `@ParametrizedTest` and `@MethodSource` annotations >> - Some test formatting unification - keeping annotations on a separate line >> from method definition >> - Some tests used `@Rule` annotation which is missing from JUnit5. Luckily, >> all those uses just set `ExpectedException thrown = >> ExpectedException.none()` and then mid-test set it to some expected >> exception class. This was easily replaceable with `assertThrows`. >> >> Non-trivial changes: >> - Mutliple tests use common test bases and derive from it to check specific >> behaviors. Those include: >> - `DirtyRegionTestBase` >> - `CssMethodsTestBase` >> - `ObjectMethodsTestBase` >> - `OnInvalidateMethodsTestBase` >> - `PropertiesTestBase` >> - `ServiceTestBase` & derivatives required some rework due to >> `ServiceExceptionTest` being a derived parametrized class while other >> `ServiceTestBase` were not parametrized >> - `TransformOperationsTest` not only expanded 3 test parameters onto 6 >> different test class members, but also exposed its `getParams()` call to >> other testers - `AffineOperationsTest` and `TransformUtilsTest` >> - Other classes used `@BeforeEach` annotation which accessed test >> parameters, those setup methods were called manually in-test as per >> discussion in #1561 >> >> At the end I added a change converting `assertTimeout` uses I added to >> `@Timeout` annotation, as recommended in other PRs. >> >> As for conversion practices and methods, most of these were discussed as >> part of #1561 so I will skip them here. >> >> ## Results >> >> Below are results of the test conversion and runs of `gradle --info >> :graphics:test` on my machine: >> >> | Category | JUnit4 ( 4647367ce ) | JUnit5 (this PR) | >> | ------------ | ------ | ------ | >> | Test count | 23272 | 23275 | >> | Failures | 0 | 0 | >> | Ignored | 99 | 102 | >> | Successful % | 100% | 100% | >> >> The difference in test count and ignored comes from >> `Popup_parentWindow_Test.java` where the entire test class was annotated as >> `@Ignored`. After conversion... > > modules/javafx.graphics/src/test/java/test/com/sun/javafx/sg/prism/DirtyRegionTestBase.java > line 76: > >> 74: return Stream.concat(stream, Stream.of(arg)); >> 75: } >> 76: > > same comment: `List<Arguments>` might also work > (here and possibly elsewhere, I don't want to add duplicate comments) There will be plenty of uses of that all throughout this change, I wasn't 100% sure what JUnit5 expects or allows so I used the same `Stream<Arguments>` return type for parameter methods across all tests. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1566#discussion_r1766506669
