On Wed, 18 Jun 2025 19:18:06 GMT, Manukumar V S <m...@openjdk.org> wrote:

>> Issue:
>> MyanmarTextTest.java produces a false positive result when some of the test 
>> preconditions are not met. It checks whether certain fonts are present in 
>> the system, for example, 'Padauk' on Linux. If the required font is missing, 
>> the test simply returns early, and the test ends up passing, which is 
>> incorrect. Ideally, it should throw a jtreg.SkippedException when the 
>> necessary preconditions are not satisfied.
>> 
>> Another scenario is that the test passes on headless machines even though it 
>> creates GUI components. Ideally, when GUI components are created in code 
>> running on a headless machine, a HeadlessException should be thrown. 
>> However, since MyanmarTextTest.java exits before reaching the point where 
>> the GUI is created (due to unmet preconditions), it incorrectly reports a 
>> pass. This behavior may lead to a misinterpretation of the test as being 
>> headless, which it is not.
>> 
>> Fix:
>> Need to throw jtreg.SkippedException in cases where some pre-conditions for 
>> running the test are not met.
>> 
>> Testing:
>> Tested using mach5 in all available platforms and got full PASS.
>
> Manukumar V S has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Added blank line before OS_NAME

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 59:

> 57:     private static final String[] FONTS_WINDOWS = {"Myanmar Text", "Noto 
> Sans Myanmar"};
> 58:     private static final String[] FONTS_LINUX = {"Padauk", "Noto Sans 
> Myanmar", "Myanmar Text"};
> 59:     private static final String[] FONTS_MACOS = {"Myanmar MN", "Noto Sans 
> Myanmar", "Myanmar Text"};

I'm unsure if Noto Sans fonts are installed by default on Windows or macOS.

Similarly, "Myanmar Text" is unlikely to be installed on Linux.

Since there's a list of possible fonts, the list could well be 
*platform-independent* — just look for a first match.

test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 61:

> 59:     private static final String[] FONTS_MACOS = {"Myanmar MN", "Noto Sans 
> Myanmar", "Myanmar Text"};
> 60: 
> 61:     private static final String[] FONT_CANDIDATES = 
> selectFontCandidates();

Suggestion:

    private static final List<String> FONT_CANDIDATES =
            List.of("Myanmar MN",
                    "Padauk",
                    "Myanmar Text",
                    "Noto Sans Myanmar");

test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 74:

> 72:             throw new SkippedException("Unsupported OS: " + OS_NAME);
> 73:         }
> 74:         if (FONT_NAME == null) {

With the changes that I propose, `FONT_NAME = selectFontName()`, this will be 
the only condition: if the font name is `null`, no suitable font is found — 
skip the test.

test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 105:

> 103:         main.setLayout(new BoxLayout(main, BoxLayout.Y_AXIS));
> 104:         main.add(myanmarTF);
> 105:         main.setBorder(BorderFactory.createEmptyBorder(7, 7, 7, 7));

I see no reason for removing this blank line.

test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 153:

> 151:     }
> 152: 
> 153:     private static String selectAvailableFont() {

private static String selectFontName() {
        return Arrays.stream(GraphicsEnvironment
                             .getLocalGraphicsEnvironment()
                             .getAvailableFontFamilyNames())
                     .filter(FONT_CANDIDATES::contains)
                     .findFirst()
                     .orElse(null);
    }

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

PR Review: https://git.openjdk.org/jdk/pull/25879#pullrequestreview-2940458706
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155365237
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155390963
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155402010
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155366028
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155392324

Reply via email to