On Wed, 1 Dec 2021 12:21:00 GMT, Athijegannathan Sundararajan 
<sun...@openjdk.org> 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

Reply via email to