Re: [v4] Fix strptime era handling, add more era tests [BZ #24394]

[DJ Delorie <dj at redhat dot com>]

Rafal Luzynski <digitalfreak@lingonborough.com> writes:
>> Added the BCE/CE transition.  I won't guarantee that the day of the week
>> is correct for that one; I used what strftime returned.  I'm not testing
>> the day of week code here ;-)
>
> Do we need to initialize any weekday number at all?  It's difficult
> to figure out the correct weekday number for past dates without using
> a calendar.

You wouldn't think so, but if I didn't, I got mismatches.  So I put the
right values in anyway.  Or at least the matching values.

Plus, this framework can test for weekday names and exceptions too.
It's not limited to eras, I just only added those so far.

>> +/* Test for strftime, esp Japenese era name changes.
>
> Is "esp" a shortcut for "especially"?  Unless I am the only one confused
> here, would you mind using the full version?

Yes.  I'll expand it.

>> [...]
>> +/* These exist for the convenience of writing the test data, because
>> +   zero-based vs one-based.  */
>> +typedef enum {
>> +  Sun, Mon, Tue, Wed, Thu, Fri, Sat
>> +} WeekDay;
>> +
>> +typedef enum {
>> +  Jan, Feb, Mar, Apr, May, Jun, Jul, Aug, Sep, Oct, Nov, Dec
>> +} Month;
>
> This pattern is excellent.  I hope you don't mind that I use for
> the other tests for strftime() which I'm currently working on.

Please do :-)

(or just add more data to my tables ;)

>> +  { "Japanese era change, BCE/CE, before transition",
>> +    0, Dec,  31, Sun, 12, 00, 00, "ja_JP.UTF-8", "%EY",
>
> Do we need two ^ spaces here and three spaces in the dates where
> a day number is one-digit?  If not, can you please remove one space?

Done.  Wow, I got that wrong a lot ;-)

>> +  snprintf (buffer, TMBUFLEN, "%04d/%02d/%02d-%02d:%02d:%02d-%d",
>> +	    tm->tm_year,
>
> Would it be helpful to print the weekday name (even abbreviated)
> instead of a weekday number which may be confusing? Suggestion:
>
> const char *weekday_name[] = { "Sun", "Mon", "Tue", "Wed", "Thu", "Fri",
> "Sat" };

I got halfway through adding that, then my brain said "don't assume the
weekday returned will be valid".  Then it got complicated.  I think I'd
prefer leaving it a digit.

>   snprintf (buffer, TMBUFLEN, "%04d/%02d/%02d-%02d:%02d:%02d, %s",
> What about a space rather than a dash between a day number and an hour?

Ok.

>> +static int
>> +do_test (void)
>> +{
>> [...]
>> +      /* Print this just to help debug failures.  */
>> +      printf("%s: %s %s %s\n", d->name, d->locale, d->format,
>> d->printed);
>
> This often produces the lines which exceed 80 columns.  Can we
> have a linebreak in the middle?  Suggestions:

Can we PLEASE switch to 132-column VT100's already?  ;-)

I'll do something.  More likely I'll tweak the test names to keep it
under 80; with a lot of tests, one per line is an easy pattern to see.

Maybe I'll get clever about it, too.

> Will it be easy to add more locales and more calendars to this test file?
> I think it is possible but I'm also thinking on whether it can be easier.

Yes!  Each entry in the table specifies everything about the test,
including the locale.  It makes adding tests a bit tedious but it's very
flexible.  The only feature it doesn't have is a flag for testing *only*
strftime or *only* strptime, like for testing forced-error cases.  Easy
to add that if it's needed.

If the tables get big, we could split them into .h files, too.