On Wed, 18 Jun 2025 19:18:06 GMT, Manukumar V S <[email protected]> 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
