RFR: 8273685 : Remove jtreg tag manual=yesno for java/awt/Graphics/LCDTexTAndGraphicsState.java [v2]

Alexey Ivanov aivanov at openjdk.java.net
Wed Sep 15 15:32:47 UTC 2021


On Tue, 14 Sep 2021 00:10:26 GMT, lawrence.andrews <github.com+87324768+lawrence-andrew at openjdk.org> wrote:

>> Problem : 
>> 1) Test case was failing with following exception
>> test result: Error. Parse Exception: Arguments to `manual' option not supported: yes 
>> 2) After removing =yes/no Parser exception is not seen but test UI will show and closes immediately  where the user cannot see what is on the frame. 
>> 
>> Fix : 
>> 1) Add frame to show the test instructions.
>> 2) Added timeout for the test case.
>> 3) Show the test UI until either test case gets timeout or user press pass or fail button.
>> 
>> @shurymury
>
> lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed minor issue

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 59:

> 57: 
> 58:     private static final Frame instructionFrame = new Frame();
> 59:     private static volatile boolean testResult = false;

Assigning the default value is redundant.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 60:

> 58:     private static final Frame instructionFrame = new Frame();
> 59:     private static volatile boolean testResult = false;
> 60:     private static volatile CountDownLatch countDownLatch;

It could be `final` and could be initialised here rather than in main.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 61:

> 59:     private static volatile boolean testResult = false;
> 60:     private static volatile CountDownLatch countDownLatch;
> 61:     private static String text = "This test passes only if this text appears SIX TIMES";

Could be declared `final`.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 79:

> 77:         Graphics2D g2d = (Graphics2D) g;
> 78:         g2d.setRenderingHint(RenderingHints.KEY_TEXT_ANTIALIASING,
> 79:                 RenderingHints.VALUE_TEXT_ANTIALIAS_LCD_HRGB);

I'd rather keep them aligned as they were. I'm fine with reformatting, though. Both variants are acceptable and there's no strong preference to either in JDK code as far as I'm aware.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 114:

> 112: 
> 113:     private static void createInstructionUI() {
> 114:         GridBagLayout layout = new GridBagLayout();

I believe the UI layout could be simplified by using `BorderLayout`, the default for `Frame`, where textArea is placed as `BorderLayout.CENTER` and button panel (with BoxLayout or FlowLayout) is placed as `BorderLayout.SOUTH`.

It's just a tip. The layout works; don't fix it if it's not broken.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 137:

> 135: 
> 136:         Button failButton = new Button("Fail");
> 137:         failButton.setActionCommand("Fail");

You don't use action command, so it's somewhat redundant.

test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 162:

> 160:             }
> 161:         });
> 162:         instructionFrame.pack();

`pack()` is called twice.

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

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



More information about the client-libs-dev mailing list