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

Reply via email to