On Fri, 20 Dec 2019 17:55:04 GMT, Frederic Thevenet <github.com+7450507+ftheve...@openjdk.org> wrote:
>> This will need two reviewers. I want to review it, and I request @arapte to >> also review. >> >> I won't have time to do a detailed review until the new year. One quick >> comment: in addition to the new tests you have provided, there are 4 >> `@Ignore`d tests in >> [Snapshot2Test.java](https://github.com/openjdk/jfx/blob/master/tests/system/src/test/java/test/javafx/scene/Snapshot2Test.java) >> that can likely be re-enabled. Look for `TODO: Re-enable this test when >> RT-22073 is fixed` (RT-22073 was mapped to JDK-8088198). > >> >> >> This will need two reviewers. I want to review it, and I request @arapte to >> also review. >> >> I won't have time to do a detailed review until the new year. One quick >> comment: in addition to the new tests you have provided, there are 4 >> `@Ignore`d tests in >> [Snapshot2Test.java](https://github.com/openjdk/jfx/blob/master/tests/system/src/test/java/test/javafx/scene/Snapshot2Test.java) >> that can likely be re-enabled. Look for `TODO: Re-enable this test when >> RT-22073 is fixed` (RT-22073 was mapped to JDK-8088198). > > I hadn't noticed these tests before, but at a glance it does indeed look like > they make the ones I added redundant. Upon closer inspection, I believe that the tests I added in Snapshot3Test are indeed redundant and less complete than the 4 ignored test in Snapshot2Test. I therefore propose to remove Snapshot3Test.java entirely and to re-enable the 4 testSnapshotBig* test instead. ------------- PR: https://git.openjdk.java.net/jfx/pull/68