Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-12-01 Thread Jaikiran Pai
On Wed, 1 Dec 2021 12:21:00 GMT, Athijegannathan Sundararajan 
 wrote:

>> Can I please get a review of this change which adds `jlink.debug=true` 
>> system property while launching `jpackage` tests?
>> 
>> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
>> take into account the part where the `jpackage` tool gets launched as a 
>> `ToolProvider` from some of these tests. This resulted in a large number of 
>> tests failing (across different OS) in `tier2` with errors like:
>> 
>> 
>> Error: Invalid Option: [-J-Djlink.debug=true]
>> 
>> 
>> In this current PR, the changed code now takes into account the possibility 
>> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't 
>> add this option. To achieve this, the adding of this argument is delayed 
>> till when the actual execution is about to happen and thus it's now done in 
>> the `adjustArgumentsBeforeExecution()` method of the jpackage test framework.
>> 
>> With this change, I have now run the `jdk:tier2` locally on a macos instance 
>> and the tests have all passed:
>> 
>> 
>> Test results: passed: 3,821; failed: 3
>> Report written to 
>> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
>> Results written to 
>> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
>> Error: Some tests failed or other problems occurred.
>> Finished running test 'jtreg:test/jdk:tier2'
>> Test report is stored in 
>> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2
>> 
>> ==
>> Test summary
>> ==
>>TEST  TOTAL  PASS  FAIL ERROR 
>>   
 jtreg:test/jdk:tier2   3824  3821 3 0 
 <<
>> ==
>> 
>> The 3 failing tests are unrelated to this change and belong to the 
>> `java/nio/channels/DatagramChannel/` test package.
>> Furthermore, I've looked into the generated logs of the following tests to 
>> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
>> doesn't in those that failed previously in `tier2`:
>> 
>> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
>> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
>> test/jdk/tools/jpackage/share/ArgumentsTest.java
>> 
>> A sample from one of the logs where this system property is expected to be 
>> passed along:
>> 
>>> TRACE: exec: Execute 
>>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
>>> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
>>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true 
>>> --verbose](15); inherit I/O...
>> 
>> 
>> I would still like to request someone with access to CI or other OSes (like 
>> Windows and Linux) to help test `tier2` against this PR.
>
> LGTM

Thank you for the review @sundararajana

-

PR: https://git.openjdk.java.net/jdk/pull/6534


Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-12-01 Thread Athijegannathan Sundararajan
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which adds `jlink.debug=true` system 
> property while launching `jpackage` tests?
> 
> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
> take into account the part where the `jpackage` tool gets launched as a 
> `ToolProvider` from some of these tests. This resulted in a large number of 
> tests failing (across different OS) in `tier2` with errors like:
> 
> 
> Error: Invalid Option: [-J-Djlink.debug=true]
> 
> 
> In this current PR, the changed code now takes into account the possibility 
> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't 
> add this option. To achieve this, the adding of this argument is delayed till 
> when the actual execution is about to happen and thus it's now done in the 
> `adjustArgumentsBeforeExecution()` method of the jpackage test framework.
> 
> With this change, I have now run the `jdk:tier2` locally on a macos instance 
> and the tests have all passed:
> 
> 
> Test results: passed: 3,821; failed: 3
> Report written to 
> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
> Results written to 
> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
> Error: Some tests failed or other problems occurred.
> Finished running test 'jtreg:test/jdk:tier2'
> Test report is stored in 
> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier2   3824  3821 3 0 <<
> ==
> 
> The 3 failing tests are unrelated to this change and belong to the 
> `java/nio/channels/DatagramChannel/` test package.
> Furthermore, I've looked into the generated logs of the following tests to 
> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
> doesn't in those that failed previously in `tier2`:
> 
> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
> test/jdk/tools/jpackage/share/ArgumentsTest.java
> 
> A sample from one of the logs where this system property is expected to be 
> passed along:
> 
>> TRACE: exec: Execute 
>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
>> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); 
>> inherit I/O...
> 
> 
> I would still like to request someone with access to CI or other OSes (like 
> Windows and Linux) to help test `tier2` against this PR.

LGTM

-

Marked as reviewed by sundar (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6534


Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-12-01 Thread Jaikiran Pai
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which adds `jlink.debug=true` system 
> property while launching `jpackage` tests?
> 
> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
> take into account the part where the `jpackage` tool gets launched as a 
> `ToolProvider` from some of these tests. This resulted in a large number of 
> tests failing (across different OS) in `tier2` with errors like:
> 
> 
> Error: Invalid Option: [-J-Djlink.debug=true]
> 
> 
> In this current PR, the changed code now takes into account the possibility 
> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't 
> add this option. To achieve this, the adding of this argument is delayed till 
> when the actual execution is about to happen and thus it's now done in the 
> `adjustArgumentsBeforeExecution()` method of the jpackage test framework.
> 
> With this change, I have now run the `jdk:tier2` locally on a macos instance 
> and the tests have all passed:
> 
> 
> Test results: passed: 3,821; failed: 3
> Report written to 
> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
> Results written to 
> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
> Error: Some tests failed or other problems occurred.
> Finished running test 'jtreg:test/jdk:tier2'
> Test report is stored in 
> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier2   3824  3821 3 0 <<
> ==
> 
> The 3 failing tests are unrelated to this change and belong to the 
> `java/nio/channels/DatagramChannel/` test package.
> Furthermore, I've looked into the generated logs of the following tests to 
> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
> doesn't in those that failed previously in `tier2`:
> 
> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
> test/jdk/tools/jpackage/share/ArgumentsTest.java
> 
> A sample from one of the logs where this system property is expected to be 
> passed along:
> 
>> TRACE: exec: Execute 
>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
>> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); 
>> inherit I/O...
> 
> 
> I would still like to request someone with access to CI or other OSes (like 
> Windows and Linux) to help test `tier2` against this PR.

Any help reviewing this please?

-

PR: https://git.openjdk.java.net/jdk/pull/6534


Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Alexey Semenyuk



On 11/24/2021 8:35 AM, Andy Herrick wrote:
Wait - the original change, 'addArgument("-J-Djlink.debug=true");' 
adds "-D-J..." as a jpackage arg.  This is not a supported command 
line option, I'm not sure why it is not throwing an error all the 
time, but you need to use addArgument("--java-option 
-Djlink.debug=true" ); instead.
This would add -Djlink.debug=true to java options of an app, right? But 
we need -Djlink.debug=true for jlink launched from jpackage.


- Alexey


I'll look into why "-J..." is being silently ignored (at least on 
windows) and not throwing a invalid argument exception.


/Andy


On 11/24/2021 4:03 AM, Jaikiran Pai wrote:
Can I please get a review of this change which adds 
`jlink.debug=true` system property while launching `jpackage` tests?


The previous fix for this in https://github.com/openjdk/jdk/pull/6491 
didn't take into account the part where the `jpackage` tool gets 
launched as a `ToolProvider` from some of these tests. This resulted 
in a large number of tests failing (across different OS) in `tier2` 
with errors like:



Error: Invalid Option: [-J-Djlink.debug=true]


In this current PR, the changed code now takes into account the 
possibility of `jpackage` being launched as a `ToolProvider` and in 
such cases doesn't add this option. To achieve this, the adding of 
this argument is delayed till when the actual execution is about to 
happen and thus it's now done in the 
`adjustArgumentsBeforeExecution()` method of the jpackage test 
framework.


With this change, I have now run the `jdk:tier2` locally on a macos 
instance and the tests have all passed:



Test results: passed: 3,821; failed: 3
Report written to 
jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
Results written to 
jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2

Error: Some tests failed or other problems occurred.
Finished running test 'jtreg:test/jdk:tier2'
Test report is stored in 
build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2


==
Test summary
==
    TEST  TOTAL PASS  
FAIL ERROR

jtreg:test/jdk:tier2 3824  3821 3 0 <<

==

The 3 failing tests are unrelated to this change and belong to the 
`java/nio/channels/DatagramChannel/` test package.
Furthermore, I've looked into the generated logs of the following 
tests to verify that the `-J-Djlink.debug=true` does get passed in 
relevant tests and doesn't in those that failed previously in `tier2`:


test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
test/jdk/tools/jpackage/share/ArgumentsTest.java

A sample from one of the logs where this system property is expected 
to be passed along:


TRACE: exec: Execute 
[jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage 
--input ./test/input --dest ./test/output --name "Name With Space" 
--type pkg --main-jar hello.jar --main-class Hello 
-J-Djlink.debug=true --verbose](15); inherit I/O...


I would still like to request someone with access to CI or other OSes 
(like Windows and Linux) to help test `tier2` against this PR.


-

Commit messages:
  - 8277647: [REDO] JDK-8277507 Add jlink.debug system property while 
launching jpackage tests to help diagonize recent intermittent failures


Changes: https://git.openjdk.java.net/jdk/pull/6534/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6534=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8277647
   Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/6534.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk 
pull/6534/head:pull/6534


PR: https://git.openjdk.java.net/jdk/pull/6534




Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Jaikiran Pai

Hello Andy,

On 24/11/21 7:23 pm, Andy Herrick wrote:
never mind my previous  email, addArgument("--java-option ...") , 
would be for argument to the java launching the app.


you are right to use -J-Djlink.debug=true .  It must be interpreted in 
the standard launcher used for jpackage, because the -J-D arg never 
gets to the jpackage java code and the System Property "jlink.debug" 
is already set to true by then.


Thank you for the clarification and verifying the value. I did a similar 
(manual) check and like you say, the system property does rightly make 
it to the JLinkTask.


-Jaikiran



Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Andy Herrick
never mind my previous  email, addArgument("--java-option ...") , would 
be for argument to the java launching the app.


you are right to use -J-Djlink.debug=true .  It must be interpreted in 
the standard launcher used for jpackage, because the -J-D arg never gets 
to the jpackage java code and the System Property "jlink.debug" is 
already set to true by then.


/Andy

On 11/24/2021 8:35 AM, Andy Herrick wrote:
Wait - the original change, 'addArgument("-J-Djlink.debug=true");' 
adds "-D-J..." as a jpackage arg.  This is not a supported command 
line option, I'm not sure why it is not throwing an error all the 
time, but you need to use addArgument("--java-option 
-Djlink.debug=true" ); instead.


I'll look into why "-J..." is being silently ignored (at least on 
windows) and not throwing a invalid argument exception.


/Andy


On 11/24/2021 4:03 AM, Jaikiran Pai wrote:
Can I please get a review of this change which adds 
`jlink.debug=true` system property while launching `jpackage` tests?


The previous fix for this in https://github.com/openjdk/jdk/pull/6491 
didn't take into account the part where the `jpackage` tool gets 
launched as a `ToolProvider` from some of these tests. This resulted 
in a large number of tests failing (across different OS) in `tier2` 
with errors like:



Error: Invalid Option: [-J-Djlink.debug=true]


In this current PR, the changed code now takes into account the 
possibility of `jpackage` being launched as a `ToolProvider` and in 
such cases doesn't add this option. To achieve this, the adding of 
this argument is delayed till when the actual execution is about to 
happen and thus it's now done in the 
`adjustArgumentsBeforeExecution()` method of the jpackage test 
framework.


With this change, I have now run the `jdk:tier2` locally on a macos 
instance and the tests have all passed:



Test results: passed: 3,821; failed: 3
Report written to 
jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
Results written to 
jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2

Error: Some tests failed or other problems occurred.
Finished running test 'jtreg:test/jdk:tier2'
Test report is stored in 
build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2


==
Test summary
==
    TEST  TOTAL PASS  
FAIL ERROR

jtreg:test/jdk:tier2 3824  3821 3 0 <<

==

The 3 failing tests are unrelated to this change and belong to the 
`java/nio/channels/DatagramChannel/` test package.
Furthermore, I've looked into the generated logs of the following 
tests to verify that the `-J-Djlink.debug=true` does get passed in 
relevant tests and doesn't in those that failed previously in `tier2`:


test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
test/jdk/tools/jpackage/share/ArgumentsTest.java

A sample from one of the logs where this system property is expected 
to be passed along:


TRACE: exec: Execute 
[jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage 
--input ./test/input --dest ./test/output --name "Name With Space" 
--type pkg --main-jar hello.jar --main-class Hello 
-J-Djlink.debug=true --verbose](15); inherit I/O...


I would still like to request someone with access to CI or other OSes 
(like Windows and Linux) to help test `tier2` against this PR.


-

Commit messages:
  - 8277647: [REDO] JDK-8277507 Add jlink.debug system property while 
launching jpackage tests to help diagonize recent intermittent failures


Changes: https://git.openjdk.java.net/jdk/pull/6534/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6534=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8277647
   Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/6534.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk 
pull/6534/head:pull/6534


PR: https://git.openjdk.java.net/jdk/pull/6534


Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Andy Herrick
Wait - the original change, 'addArgument("-J-Djlink.debug=true");' adds 
"-D-J..." as a jpackage arg.  This is not a supported command line 
option, I'm not sure why it is not throwing an error all the time, but 
you need to use addArgument("--java-option -Djlink.debug=true" ); instead.


I'll look into why "-J..." is being silently ignored (at least on 
windows) and not throwing a invalid argument exception.


/Andy


On 11/24/2021 4:03 AM, Jaikiran Pai wrote:

Can I please get a review of this change which adds `jlink.debug=true` system 
property while launching `jpackage` tests?

The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
take into account the part where the `jpackage` tool gets launched as a 
`ToolProvider` from some of these tests. This resulted in a large number of 
tests failing (across different OS) in `tier2` with errors like:


Error: Invalid Option: [-J-Djlink.debug=true]


In this current PR, the changed code now takes into account the possibility of 
`jpackage` being launched as a `ToolProvider` and in such cases doesn't add 
this option. To achieve this, the adding of this argument is delayed till when 
the actual execution is about to happen and thus it's now done in the 
`adjustArgumentsBeforeExecution()` method of the jpackage test framework.

With this change, I have now run the `jdk:tier2` locally on a macos instance 
and the tests have all passed:


Test results: passed: 3,821; failed: 3
Report written to 
jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
Results written to 
jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
Error: Some tests failed or other problems occurred.
Finished running test 'jtreg:test/jdk:tier2'
Test report is stored in 
build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2

==
Test summary
==
TEST  TOTAL  PASS  FAIL ERROR

jtreg:test/jdk:tier2   3824  3821 3 0 <<

==

The 3 failing tests are unrelated to this change and belong to the 
`java/nio/channels/DatagramChannel/` test package.
Furthermore, I've looked into the generated logs of the following tests to 
verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
doesn't in those that failed previously in `tier2`:

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
test/jdk/tools/jpackage/share/ArgumentsTest.java

A sample from one of the logs where this system property is expected to be 
passed along:


TRACE: exec: Execute [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage 
--input ./test/input --dest ./test/output --name "Name With Space" --type pkg 
--main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); inherit 
I/O...


I would still like to request someone with access to CI or other OSes (like 
Windows and Linux) to help test `tier2` against this PR.

-

Commit messages:
  - 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching 
jpackage tests to help diagonize recent intermittent failures

Changes: https://git.openjdk.java.net/jdk/pull/6534/files
  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6534=00
   Issue: https://bugs.openjdk.java.net/browse/JDK-8277647
   Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
   Patch: https://git.openjdk.java.net/jdk/pull/6534.diff
   Fetch: git fetch https://git.openjdk.java.net/jdk pull/6534/head:pull/6534

PR: https://git.openjdk.java.net/jdk/pull/6534


Re: RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Kevin Rushforth
On Wed, 24 Nov 2021 08:54:08 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which adds `jlink.debug=true` system 
> property while launching `jpackage` tests?
> 
> The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
> take into account the part where the `jpackage` tool gets launched as a 
> `ToolProvider` from some of these tests. This resulted in a large number of 
> tests failing (across different OS) in `tier2` with errors like:
> 
> 
> Error: Invalid Option: [-J-Djlink.debug=true]
> 
> 
> In this current PR, the changed code now takes into account the possibility 
> of `jpackage` being launched as a `ToolProvider` and in such cases doesn't 
> add this option. To achieve this, the adding of this argument is delayed till 
> when the actual execution is about to happen and thus it's now done in the 
> `adjustArgumentsBeforeExecution()` method of the jpackage test framework.
> 
> With this change, I have now run the `jdk:tier2` locally on a macos instance 
> and the tests have all passed:
> 
> 
> Test results: passed: 3,821; failed: 3
> Report written to 
> jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
> Results written to 
> jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
> Error: Some tests failed or other problems occurred.
> Finished running test 'jtreg:test/jdk:tier2'
> Test report is stored in 
> build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2
> 
> ==
> Test summary
> ==
>TEST  TOTAL  PASS  FAIL ERROR  
>  
>>> jtreg:test/jdk:tier2   3824  3821 3 0 <<
> ==
> 
> The 3 failing tests are unrelated to this change and belong to the 
> `java/nio/channels/DatagramChannel/` test package.
> Furthermore, I've looked into the generated logs of the following tests to 
> verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
> doesn't in those that failed previously in `tier2`:
> 
> test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
> test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
> test/jdk/tools/jpackage/share/ArgumentsTest.java
> 
> A sample from one of the logs where this system property is expected to be 
> passed along:
> 
>> TRACE: exec: Execute 
>> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
>> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
>> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); 
>> inherit I/O...
> 
> 
> I would still like to request someone with access to CI or other OSes (like 
> Windows and Linux) to help test `tier2` against this PR.

@sashamatveev should be able to run the tier2 tests for you as part of his 
(re)reviewing the fix.

-

PR: https://git.openjdk.java.net/jdk/pull/6534


RFR: 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching jpackage tests to help diagonize recent intermittent failures

2021-11-24 Thread Jaikiran Pai
Can I please get a review of this change which adds `jlink.debug=true` system 
property while launching `jpackage` tests?

The previous fix for this in https://github.com/openjdk/jdk/pull/6491 didn't 
take into account the part where the `jpackage` tool gets launched as a 
`ToolProvider` from some of these tests. This resulted in a large number of 
tests failing (across different OS) in `tier2` with errors like:


Error: Invalid Option: [-J-Djlink.debug=true]


In this current PR, the changed code now takes into account the possibility of 
`jpackage` being launched as a `ToolProvider` and in such cases doesn't add 
this option. To achieve this, the adding of this argument is delayed till when 
the actual execution is about to happen and thus it's now done in the 
`adjustArgumentsBeforeExecution()` method of the jpackage test framework.

With this change, I have now run the `jdk:tier2` locally on a macos instance 
and the tests have all passed:


Test results: passed: 3,821; failed: 3
Report written to 
jdk/build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2/html/report.html
Results written to 
jdk/build/macosx-x86_64-server-release/test-support/jtreg_test_jdk_tier2
Error: Some tests failed or other problems occurred.
Finished running test 'jtreg:test/jdk:tier2'
Test report is stored in 
build/macosx-x86_64-server-release/test-results/jtreg_test_jdk_tier2

==
Test summary
==
   TEST  TOTAL  PASS  FAIL ERROR   
>> jtreg:test/jdk:tier2   3824  3821 3 0 <<
==

The 3 failing tests are unrelated to this change and belong to the 
`java/nio/channels/DatagramChannel/` test package.
Furthermore, I've looked into the generated logs of the following tests to 
verify that the `-J-Djlink.debug=true` does get passed in relevant tests and 
doesn't in those that failed previously in `tier2`:

test/jdk/tools/jpackage/share/MultiLauncherTwoPhaseTest.java
test/jdk/tools/jpackage/macosx/NameWithSpaceTest.java
test/jdk/tools/jpackage/share/ArgumentsTest.java

A sample from one of the logs where this system property is expected to be 
passed along:

> TRACE: exec: Execute 
> [jdk/build/macosx-x86_64-server-release/images/jdk/bin/jpackage --input 
> ./test/input --dest ./test/output --name "Name With Space" --type pkg 
> --main-jar hello.jar --main-class Hello -J-Djlink.debug=true --verbose](15); 
> inherit I/O...


I would still like to request someone with access to CI or other OSes (like 
Windows and Linux) to help test `tier2` against this PR.

-

Commit messages:
 - 8277647: [REDO] JDK-8277507 Add jlink.debug system property while launching 
jpackage tests to help diagonize recent intermittent failures

Changes: https://git.openjdk.java.net/jdk/pull/6534/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6534=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277647
  Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6534.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6534/head:pull/6534

PR: https://git.openjdk.java.net/jdk/pull/6534