On Fri, 16 Apr 2021 02:35:23 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> test/jdk/java/util/Calendar/NarrowNamesTest.java line 115:
>> 
>>> 113:         } else {
>>> 114:             testMap(US, AM_PM, ALL_STYLES,
>>> 115:                     "AM", "PM",
>> 
>> What I meant was there is no need to check the providers and introduce the 
>> am/pm specific test method. Both `CLDR` and `COMPAT` (i.e., no "if" 
>> statement) should be tested with `testMap()` method as other tests do.
>
> Hello Naoto,
> 
> As far as I can see, the testMap cannot be used to test the day period 
> strings. More specifically, consider this change (git diff) that I did as you 
> suggested (unless I misunderstood what you meant):
> 
> 
> +        testMap(US, AM_PM, ALL_STYLES,
> +                "AM",
> +                "PM",
> +                "midnight",
> +                "noon",
> +                "in the morning",
> +                "",
> +                "in the afternoon",
> +                "",
> +                "in the evening",
> +                "",
> +                "at night",
> +                "",
> +                RESET_INDEX,
> +                "a", "p", "mi", "n", "", "", "", "", "", "", "", "");
> +        testMap(US, AM_PM, ALL_STYLES,
> +                "AM", "PM",
> +                RESET_INDEX,
> +                "a", "p");
> 
> 
> 
> This will fail with the following error.
> 
> testMap: locale=en_US, field=9, style=0, expected={in the afternoon=6, in the 
> evening=8, in the morning=4, at night=10, midnight=2, noon=3, AM=0, PM=1, 
> mi=2, a=0, n=3, p=1}, got={AM=0, PM=1, a=0, p=1}
> java.lang.RuntimeException: test failed
> 
> 
> That failure is due to the `testMap` method expecting these day period string 
> to be part of the returned Map from `Calendar.getDisplayNames()` which won't 
> be the case because that API (as per the javadoc) will only return a Map 
> whose display names have valid field values, so in this case only those 
> display names which will have `AM` or `PM` as a value.
> 
> If it's easier to review, I will go ahead and push the suggested change in 
> this testcase and let if fail so that you can get a chance to look at the 
> actual test code and error. Let me know.

Sorry if I wasn't clear. The whole test for am/pm (line 98 - 118) can simply be:

            testMap(US, AM_PM, ALL_STYLES,
                    "AM", "PM",
                    RESET_INDEX,
                    "a", "p");

Since we now know that day periods strings won't be leaked into display names, 
then the map simply should contain only 4 entries (`AM`, `PM`, `a`, and `p`).

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

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

Reply via email to