On Fri, 29 Aug 2025 02:48:35 GMT, SendaoYan <s...@openjdk.org> wrote:

>> Hi all,
>> 
>> I think test javax/print/PrintServiceLookup/CountPrintServices.java should 
>> throw jtreg.SkippedException when there is no lpstat, rather that report 
>> test failure.
>> 
>> Only javax/print/PrintServiceLookup/CountPrintServices.java invokes lpstat 
>> explicitly.
>> 
>> Change has been verified locally, test-fix only, no risk.
>
> SendaoYan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use "@requires (os.family == "linux")" instead of check OS type in code

Changes requested by aivanov (Reviewer).

test/jdk/javax/print/PrintServiceLookup/CountPrintServices.java line 53:

> 51:         proc = Runtime.getRuntime().exec(lpcmd);
> 52:     } catch (java.io.IOException e) {
> 53:         if(e.getMessage().contains("No such file or directory")) {

Suggestion:

        if (e.getMessage().contains("No such file or directory")) {

A space after the `if` keyword.

test/jdk/javax/print/PrintServiceLookup/CountPrintServices.java line 54:

> 52:     } catch (java.io.IOException e) {
> 53:         if(e.getMessage().contains("No such file or directory")) {
> 54:             throw new SkippedException("Can not find lpstat, test skip");

Suggestion:

            throw new SkippedException("Cannot find lpstat");

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

PR Review: https://git.openjdk.org/jdk/pull/26988#pullrequestreview-3167896947
PR Review Comment: https://git.openjdk.org/jdk/pull/26988#discussion_r2309559892
PR Review Comment: https://git.openjdk.org/jdk/pull/26988#discussion_r2309561637

Reply via email to