RFR: 8279995: jpackage --add-launcher option should allow overriding description [v2]

Alexander Matveev almatvee at openjdk.java.net
Tue Feb 15 00:59:04 UTC 2022


On Fri, 11 Feb 2022 22:11:44 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

>> Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8279995: jpackage --add-launcher option should allow overriding description [v2]
>
> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java line 90:
> 
>> 88:                 .findFirst().orElse(null);
>> 89: 
>> 90:         return entry == null ? null : entry.getValue();
> 
> This can be simply
> `rawProperties.stream().findAny(item -> item.getKey().equals(key)).map(e -> e.getValue()).orElse(null);`
> 
> Or slightly better:
> 
> public String getRawPropertyValue(String key, Supplier<String> getDefault) {
>     return rawProperties.stream().findAny(item -> item.getKey().equals(key)).map(e -> e.getValue()).orElseGet(getDefault);
> }
> 
> 
> 
> Then we can create a function that will return the expected description of an additional launcher:
> 
> private String getDesciption(JPackageCommand cmd) {
>     return getRawPropertyValue("description", () -> cmd.getArgumentValue("--description", unused -> "Unknown"));
> }
> 
> 
> This will let you avoid `if (expectedDescription != null)` checks and **always** verify launcher description.

Fixed as suggested, except app name is used instead of "Unknown", since our default description is app name.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/AdditionalLauncher.java line 275:
> 
>> 273:                                     expectedDescription.equals(lines.get(i).trim());
>> 274:                         }
>> 275:                     }
> 
> This piece of code can be placed in `WindowsHelper.getExecutableDesciption(Path pathToExeFile)` function to make it reusable in other tests if needed. This way it can be used to verify the description of the main launcher.
> 
> `if (lines != null)` check is rudimentary.
> 
> Instead of running the loop, I'd simply check the output second from the last line for "---" and if so, would return the last line of the output. Otherwise would throw an exception indicating the command returned an unexpected result.

I kept original implementation. Actually, powershell returns 6 lines: empty, FileDescription, ----, Actual Description, empty, empty. I think it is better to make sure FileDescription is present and go from top to bottom in case if output can be something like: empty, ErrorMessage, ---, Actual error message, empty, empty.

-------------

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


More information about the core-libs-dev mailing list