RFR: 8273685 : Remove jtreg tag manual=yesno for java/awt/Graphics/LCDTextAndGraphicsState.java & show test instruction [v2]
lawrence.andrews
github.com+87324768+lawrence-andrew at openjdk.java.net
Fri Sep 17 11:59:47 UTC 2021
On Wed, 15 Sep 2021 15:14:36 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:
>> lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fixed minor issue
>
> 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.
Both final & volatile can be used it gives error "Illegal combination of modifiers: volatile & final"
> 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`.
Declared text as final
> 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.
Simplified the TestUI. Right just one frame is shown with UI to verify and button to decide whether test is pass or fail.
> 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.
Removed actionCommand
> test/jdk/java/awt/Graphics/LCDTextAndGraphicsState.java line 162:
>
>> 160: }
>> 161: });
>> 162: instructionFrame.pack();
>
> `pack()` is called twice.
Removed pack()
-------------
PR: https://git.openjdk.java.net/jdk/pull/5503
More information about the client-libs-dev
mailing list