RFR: 8320343 : Generate GIF images for AbstractButton/5049549/bug5049549.java [v4]

[Alexey Ivanov]

On Thu, 21 Dec 2023 04:24:10 GMT, Renjith Kannath Pariyangad <rkannathpari at openjdk.org> wrote:

>> Hi Reviewers,
>> 
>> Updated the test and it will generate required images on the fly so storing and loading image from repo could be avoided. Please review and let me know your suggestions.
>> 
>> Regards,
>> Renjith.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reverted space befor line for minimal change

Marked as reviewed by aivanov (Reviewer).

test/jdk/javax/swing/AbstractButton/5049549/bug5049549.java line 46:

> 44: */
> 45: 
> 46: public class bug5049549 {

Since you're moving it, I suggest formatting it according to conventions used in most tests:
Suggestion:

/*
 * @test
 * @bug 5049549 7132413
 * @summary Tests that the proper icon is used for different states.
 * @library ../../regtesthelpers
 * @build Blocker
 * @run main/manual bug5049549
 */
public class bug5049549 {

That is with starting asterisk on each line, and align the last asterisk.

test/jdk/javax/swing/AbstractButton/5049549/bug5049549.java line 54:

> 52:     private static Icon RS = generateImage("RS");
> 53:     private static Icon SE = generateImage("SE");
> 54:     private static Icon PR = generateImage("PR");

I suggest marking all of them `final`:
Suggestion:

    private static final Icon DE = generateImage("DE");
    private static final Icon DI = generateImage("DI");
    private static final Icon DS = generateImage("DS");
    private static final Icon RO = generateImage("RO");
    private static final Icon RS = generateImage("RS");
    private static final Icon SE = generateImage("SE");
    private static final Icon PR = generateImage("PR");

test/jdk/javax/swing/AbstractButton/5049549/bug5049549.java line 56:

> 54:     private static Icon PR = generateImage("PR");
> 55: 
> 56:     private static Blocker blocker = new Blocker();

I'd add `final` modifier to `blocker` too.

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

PR Review: https://git.openjdk.org/jdk/pull/17029#pullrequestreview-1792731407
PR Review Comment: https://git.openjdk.org/jdk/pull/17029#discussion_r1433914096
PR Review Comment: https://git.openjdk.org/jdk/pull/17029#discussion_r1433910450
PR Review Comment: https://git.openjdk.org/jdk/pull/17029#discussion_r1433911187