RFR: 8202836 : [macosx] test java/awt/Graphics/TextAAHintsTest.java fails [v3]

lawrence.andrews duke at openjdk.java.net
Thu Feb 10 05:36:46 UTC 2022


On Tue, 8 Feb 2022 19:59:57 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Dispose the frame via EDT
>
> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 2:
> 
>> 1: package awt;
>> 2: 
> 
> Usually tests are not part of any package.

removed package

> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 52:
> 
>> 50: import javax.swing.SwingUtilities;
>> 51: 
>> 52: public class TextAAHintsTest  extends Component {
> 
> There's an extra space before `extends`.

Fixed

> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 177:
> 
>> 175:                 1. Verify that first set of text are rendered correctly.
>> 176:                 2. Second set of text are created using BufferedImage of the first text.
>> 177:                 3. Third set of text are created using VolatileImage of the first text.
> 
> “are” → “is”: the _set_ is singular.

Corrected

> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 180:
> 
>> 178:                 """;
>> 179:         TextArea instructionTextArea = new TextArea(instructions, 8, 50);
>> 180:         instructionTextArea.setEnabled(false);
> 
> Please leave it enabled but make it read-only (`setEditable(false)`). This way the text will be black and easier to read.

Done

> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 191:
> 
>> 189:         });
>> 190:         Button failButton = new Button("Fail");
>> 191:         failButton.addActionListener(e->{
> 
> Usually, there are spaces around `->` in lambda expressions.
> 
> This particular event handler isn't short. I suggest making it a static method or at least moving showing the dialog into a separate method. It'll make the code easier to read.

Fixed

> test/jdk/java/awt/Graphics/TextAAHintsTest.java line 228:
> 
>> 226:         mainThread = Thread.currentThread();
>> 227:         try {
>> 228:             mainThread.sleep(testTimeOut);
> 
> Suggestion:
> 
>             Thread.sleep(testTimeOut);
> 
> `sleep` is a static method, it's better to call it via the class name rather than an instance variable. This will resolve the IDE warning.

Replaced Thread with CountDownLatch

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

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



More information about the client-libs-dev mailing list