RFR: JDK-8233270: Add support to jtreg helpers to unpack packages

Alexey Semenyuk alexey.semenyuk at oracle.com
Sat Dec 14 00:47:55 UTC 2019


Phil,

Let me provide more details on what is going on here.

JDK-8230933 - is the fix of jpackage code assigning icons to 
main/additional launchers.
JDK-8233270 - is primarily the enhancement of jpackage test helpers to 
provide functionality needed for automated testing of the fix of 
JDK-8230933.

In order to test JDK-8230933 fix it is necessary to access contents of 
packages created by jpackage. This is normally done when packages are 
installed. However our test environment doesn't allow installing of 
packages. The alternative is to extract contents of packages without 
installing them.
So in order to implement jtreg tests for JDK-8230933 fix the 
functionality to unpack packages was needed. JDK-8233270 was filed to 
track this enhancement of jpackage test helpers. Work on JDK-8233270 
resulted in quite a big refactoring of tests and test helpers of jpackage:

Changes to jtreg test helpers:
- added support to unpack .dmg, .deb, .rpm, .msi packages.
- added AdditionalLauncher helper class to test additional launchers 
functionality of jpackage.
- added LauncherIconVerifier helper class to test how jpackage assigns 
icons to launchers (main and additional).
- numerous minor code improvements.
- bugfixes.

Changes to jtreg tests:
- all temporary directories/files created during tests runs that can 
help to investigate test failures are created in a way they are deleted 
only when tests pass and not when tests exit.
- removed unused files from test\jdk\tools\jpackage\apps\com.other folder.
- refactored additional launchers jtreg tests based on the old test 
helpers inherited from javapackager to use the new test helpers.

Logically JDK-8230933 and JDK-8233270 issues are unrelated, they got 
tangled because JDK-8230933 came first and then JDK-8233270 followed. 
The proper way to address them was put on hold JDK-8230933 fix and 
implement JDK-8233270 first. Then work on JDK-8230933 fix on top of 
JDK-8233270. However the work on these two issues was carried out in the 
sandbox with the assumption they both would be a part of cumulative 
jpackage patch rolled in jdk14. Keeping them separated was not a priority.

Hope this clarifies what is in the webrev and why JDK-8230933 and 
JDK-8233270 ended up in the same patch.

- Alexey

On 12/6/2019 3:59 PM, Phil Race wrote:
>
>
> On 12/6/19 12:51 PM, Alexey Semenyuk wrote:
>>
>>
>> On 12/5/2019 8:44 PM, Alexander Matveev wrote:
>>> Hi Alexey,
>>>
>>> 1) Remove this file:
>>> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JavaTool.java.rej
>> Done. webrev updated in place.
>>
>>> 2) Agree with Phil, they probably should be pushed as two separate 
>>> bugs.
>> Decoupling is not straightforward unfortunately: tests that cover 
>> additional launchers functionality contain fixes for both [1] and [2] 
>> CRs.
>
> So the synopsis reads like this is all about updating tests, and your 
> email says
> "go read the bugs" and nothing more.
> I think most people like to see a description of the problem and the 
> proposed solution
> written in the review request.
>
> I think the source code updates are what you need to highlight.
> And I don't think you have explained why it is so hard.
> Saying is is hard is not the same thing as explaining why.
>
> I think you should decouple unless you can provide that explanation.
> And definitely you need to provide explanation of the fix etc.
>
> -phil.
>
>> If there would be no strong arguments against pushing the fixes in a 
>> single patch, I'd rather avoid decoupling.
>>
>>> 3) Do you know how to run installer tests with new changes? It is 
>>> not clear from code.
>> Fix for [1] moved code to install packages produced by jpackage from 
>> manage_packages.sh script into jtreg helper classes.
>> If you run tests locally with test_jpackage.sh script, nothing will 
>> change for you.
>>
>> - Alexey
>>>
>>> Changes itself looks fine.
>>>
>>> Thanks,
>>> Alexander
>>>
>>> On 12/5/2019 5:33 PM, Philip Race wrote:
>>>> I don't understand the relationship between these two bugs.
>>>>
>>>> -phil
>>>>
>>>> On 12/5/19, 2:47 PM, Alexey Semenyuk wrote:
>>>>> Please review  fixes for [1] and [2]. Both target jpackage tool.
>>>>>
>>>>> The webrev is at [3].
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8233270
>>>>>
>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8230933
>>>>>
>>>>> [3] http://cr.openjdk.java.net/~asemenyuk/8233270/webrev.00/
>>>>>
>>>
>>
>



More information about the core-libs-dev mailing list