Re: [Integrated] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-18 Thread Kevin Rushforth 
Changeset: aab07a4d
Author:Arun Joseph 
Committer: Kevin Rushforth 
Date:  2019-11-18 18:17:39 +
URL:   https://git.openjdk.java.net/jfx/commit/aab07a4d

8234239: [TEST_BUG] Reenable few ignored web tests

Reviewed-by: kcr

! 
modules/javafx.web/src/test/java/test/com/sun/webkit/network/CookieManagerTest.java
! modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java
! modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java
! modules/javafx.web/src/test/java/test/javafx/scene/web/MiscellaneousTest.java

Re: [Rev 01] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-18 Thread Kevin Rushforth 
On Fri, 15 Nov 2019 23:03:46 GMT, Kevin Rushforth  wrote:

> On Fri, 15 Nov 2019 10:01:51 GMT, Arun Joseph  wrote:
> 
>> The pull request has been updated with additional changes.
>> 
>> 
>> 
>> Added commits:
>>  - ca460353: Remove ignore imports and update copyright
>> 
>> Changes:
>>   - all: https://git.openjdk.java.net/jfx/pull/42/files
>>   - new: https://git.openjdk.java.net/jfx/pull/42/files/f2f121d9..ca460353
>> 
>> Webrevs:
>>  - full: https://webrevs.openjdk.java.net/jfx/42/webrev.01
>>  - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.00-01
>> 
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
>>   Stats: 6 lines in 3 files changed: 0 ins; 3 del; 3 mod
>>   Patch: https://git.openjdk.java.net/jfx/pull/42.diff
>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42
> 
> I added some questions and one requested change below.
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 82:
> 
>> 81: }
>> 82: 
>> 83: @Test public void testGarbageCollectability() throws 
>> InterruptedException {
> 
> RT-26710, aka 
> [JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139), is still 
> listed as open. Since these tests no longer hang, can you close 
> [JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139) as "Cannot 
> reproduce"?
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 58:
> 
>> 57: 
>> 58: @Test public void testOleg() throws InterruptedException{
>> 59: final String URL = new 
>> File("src/test/resources/test/html/guimark2-vector.html").toURI().toASCIIString();
> 
> This test takes 80 seconds (`16 * 5 * 1sec`) to run, which is too long for a 
> simple unit test, when the entire rest of the test suite takes about 5 
> minutes. I recommend decreasing the duration of the key frame to 200 msec, 
> which would allow the test to run in 16 seconds.
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java line 
> 90:
> 
>> 89: 
>> 90: @Test public void testDefaultPopup() {
>> 91: clear();
> 
> This test, along with `testCustomPopup` is listed as unstable. Have you run 
> it multiple times on different systems?
> 
> 
> 
> Changes requested by kcr (Lead).

OK. In that case, please close RT-34508, aka 
[JDK-8090082](https://bugs.openjdk.java.net/browse/JDK-8090082), as "Cannot 
reproduce".

PR: https://git.openjdk.java.net/jfx/pull/42

Re: [Approved] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-18 Thread Kevin Rushforth 
On Sun, 17 Nov 2019 12:08:44 GMT, Arun Joseph  wrote:

> The pull request has been updated with additional changes.
> 
> 
> 
> Added commits:
>  - 8c2b988d: Modified LeakTest.testOleg
> 
> Changes:
>   - all: https://git.openjdk.java.net/jfx/pull/42/files
>   - new: https://git.openjdk.java.net/jfx/pull/42/files/ca460353..8c2b988d
> 
> Webrevs:
>  - full: https://webrevs.openjdk.java.net/jfx/42/webrev.02
>  - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.01-02
> 
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
>   Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
>   Patch: https://git.openjdk.java.net/jfx/pull/42.diff
>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42

Looks good.



Approved by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/42

Re: [Rev 01] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-17 Thread Arun Joseph 
On Fri, 15 Nov 2019 23:03:46 GMT, Kevin Rushforth  wrote:

> On Fri, 15 Nov 2019 10:01:51 GMT, Arun Joseph  wrote:
> 
>> The pull request has been updated with additional changes.
>> 
>> 
>> 
>> Added commits:
>>  - ca460353: Remove ignore imports and update copyright
>> 
>> Changes:
>>   - all: https://git.openjdk.java.net/jfx/pull/42/files
>>   - new: https://git.openjdk.java.net/jfx/pull/42/files/f2f121d9..ca460353
>> 
>> Webrevs:
>>  - full: https://webrevs.openjdk.java.net/jfx/42/webrev.01
>>  - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.00-01
>> 
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
>>   Stats: 6 lines in 3 files changed: 0 ins; 3 del; 3 mod
>>   Patch: https://git.openjdk.java.net/jfx/pull/42.diff
>>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42
> 
> I added some questions and one requested change below.
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 82:
> 
>> 81: }
>> 82: 
>> 83: @Test public void testGarbageCollectability() throws 
>> InterruptedException {
> 
> RT-26710, aka 
> [JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139), is still 
> listed as open. Since these tests no longer hang, can you close 
> [JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139) as "Cannot 
> reproduce"?
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 58:
> 
>> 57: 
>> 58: @Test public void testOleg() throws InterruptedException{
>> 59: final String URL = new 
>> File("src/test/resources/test/html/guimark2-vector.html").toURI().toASCIIString();
> 
> This test takes 80 seconds (`16 * 5 * 1sec`) to run, which is too long for a 
> simple unit test, when the entire rest of the test suite takes about 5 
> minutes. I recommend decreasing the duration of the key frame to 200 msec, 
> which would allow the test to run in 16 seconds.
> 
> modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java line 
> 90:
> 
>> 89: 
>> 90: @Test public void testDefaultPopup() {
>> 91: clear();
> 
> This test, along with `testCustomPopup` is listed as unstable. Have you run 
> it multiple times on different systems?
> 
> 
> 
> Changes requested by kcr (Lead).

Yes, I've run the test multiple times in all 3 platforms and it's passing 
everytime.

PR: https://git.openjdk.java.net/jfx/pull/42

Re: [Rev 02] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-17 Thread Arun Joseph 
The pull request has been updated with additional changes.



Added commits:
 - 8c2b988d: Modified LeakTest.testOleg

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/42/files
  - new: https://git.openjdk.java.net/jfx/pull/42/files/ca460353..8c2b988d

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/42/webrev.02
 - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.01-02

  Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/42.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42

PR: https://git.openjdk.java.net/jfx/pull/42

Re: [Rev 01] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-15 Thread Kevin Rushforth 
On Fri, 15 Nov 2019 10:01:51 GMT, Arun Joseph  wrote:

> The pull request has been updated with additional changes.
> 
> 
> 
> Added commits:
>  - ca460353: Remove ignore imports and update copyright
> 
> Changes:
>   - all: https://git.openjdk.java.net/jfx/pull/42/files
>   - new: https://git.openjdk.java.net/jfx/pull/42/files/f2f121d9..ca460353
> 
> Webrevs:
>  - full: https://webrevs.openjdk.java.net/jfx/42/webrev.01
>  - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.00-01
> 
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
>   Stats: 6 lines in 3 files changed: 0 ins; 3 del; 3 mod
>   Patch: https://git.openjdk.java.net/jfx/pull/42.diff
>   Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42

I added some questions and one requested change below.

modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 82:

> 81: }
> 82: 
> 83: @Test public void testGarbageCollectability() throws 
> InterruptedException {

RT-26710, aka [JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139), 
is still listed as open. Since these tests no longer hang, can you close 
[JDK-8088139](https://bugs.openjdk.java.net/browse/JDK-8088139) as "Cannot 
reproduce"?

modules/javafx.web/src/test/java/test/javafx/scene/web/LeakTest.java line 58:

> 57: 
> 58: @Test public void testOleg() throws InterruptedException{
> 59: final String URL = new 
> File("src/test/resources/test/html/guimark2-vector.html").toURI().toASCIIString();

This test takes 80 seconds (`16 * 5 * 1sec`) to run, which is too long for a 
simple unit test, when the entire rest of the test suite takes about 5 minutes. 
I recommend decreasing the duration of the key frame to 200 msec, which would 
allow the test to run in 16 seconds.

modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java line 
90:

> 89: 
> 90: @Test public void testDefaultPopup() {
> 91: clear();

This test, along with `testCustomPopup` is listed as unstable. Have you run it 
multiple times on different systems?



Changes requested by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/42

Re: [Rev 01] RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-15 Thread Arun Joseph 
The pull request has been updated with additional changes.



Added commits:
 - ca460353: Remove ignore imports and update copyright

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/42/files
  - new: https://git.openjdk.java.net/jfx/pull/42/files/f2f121d9..ca460353

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/42/webrev.01
 - incr: https://webrevs.openjdk.java.net/jfx/42/webrev.00-01

  Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
  Stats: 6 lines in 3 files changed: 0 ins; 3 del; 3 mod
  Patch: https://git.openjdk.java.net/jfx/pull/42.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42

PR: https://git.openjdk.java.net/jfx/pull/42

RFR: 8234239: [TEST_BUG] Reenable few ignored web tests

2019-11-15 Thread Arun Joseph 
The following tests can be reenabled:

CookieManager.testPutOverwriteExpired
CookieManager.testPutPurgeDomainAfterExpiry
CookieManager.testPutPurgeCookiesGlobally2
CookieManager.testPutPurgeCookiesGlobally3
CookieManager.testPutPurgeCookiesGloballyAfterExpiry
CallbackTest.testCustomPopup
CallbackTest.testDefaultPopup
LeakTest.testGarbageCollectability
LeakTest.testOleg
MiscellaneousTest.testRT30835



Commits:
 - f2f121d9: 8234239: Reenable few ignored web tests

Changes: https://git.openjdk.java.net/jfx/pull/42/files
 Webrev: https://webrevs.openjdk.java.net/jfx/42/webrev.00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8234239
  Stats: 20 lines in 4 files changed: 0 ins; 20 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/42.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/42/head:pull/42

PR: https://git.openjdk.java.net/jfx/pull/42