[tz] tzcode test suite
scs at eskimo.com
Thu Jan 14 18:13:15 UTC 2016
No problem with most of those cosmetic details, I knew there'd
be some massaging necessary to conform to the "house style".
Somehow I didn't even notice that the existing Makefile hade a
'check' target! Yes, it makes sense to fold this suite in there,
and moving the expensive tests to a separate target is a good
I'm most concerned, of course, about the tests that failed for
you. Now, 19:00 really is the right value for time 0 in EST;
DST was not active in December in 1970! But I think I remember
seeing weird failures like this, myself, which I at first
attributed to a mismatch between the just-compiled version of
localtime.c I was running versus the somewhat older tzdata files
already installed on my system. But I think later I discovered
it was because I wasn't actually hitting *any* zoneinfo files,
because the stock Makefile's TZDIR didn't point to where my
system's were. So that's the first thing I'd check. (Me, I
ended up first pointing TZDIR at /usr/share which is where my
system's files were, then later at a freshly-fetched and zicced
copy of tzdata2015g.)
And if that's it, it means that two out of the first two users
have had this same problem, meaning that more will, too, so it'd
be worth doing something to forestall it. Perhaps the test suite
could print an explicit error message if it detects that valid
zoneinfo files aren't being found. (Come to think of it, I had
already been planning to add some kind of an optionally noisy
warning to localtime.c for that case; thanks for the reminder!)
> Thanks for doing that! It should be quite helpful. I briefly looked into
> it and have some comments:
> * Do you use git? If so, the output of 'git format-patch' makes it
> easier to review proposed changes. The first attached patch attempts to
> be identical to the patch you sent, except in 'git format-patch' format.
> * The second attached patch cleans up a few minor problems I noticed
> when building the test suite:
> ** "make tests" assumed "." was in PATH.
> ** "make clean" didn't clean up the test suite.
> ** Missing NEWS item.
> ** "make CFLAGS='$(GCC_DEBUG_FLAGS)'" complained about a few missing
> attributes and unused variables and so forth.
> ** TM_ZONE and TM_GMTOFF should be consistent with private.h.
> ** Symbols that needn't be extern should be static.
> ** Global variables that aren't modified should be const.
> ** I prefer "const" after the type it modifies, for consistency with
> types like "int * const *".
> ** I prefer a function's return type to be on the previous line, so that
> there is room for decorations like "static".
> Other comments:
> * The test suite fails on my platform (Fedora 23 x86-64). Why would that
> be? The first few lines of error diagnostics are:
> ./testsuite testcases
> testcases, line 1598: localtime: expected 1969-12-31 19:00:00, got
> 1969-12-31 20:00:00
> testcases, line 1598: localtime: inverse expected 0, got -3600
> testcases, line 1599: localtime: expected 1901-12-13 15:45:52, got
> 1901-12-13 16:45:52
> testcases, line 1599: localtime: inverse expected -2147483648, got
> testcases, line 1604: localtime: expected 1969-12-31 16:00:00, got
> 1969-12-31 17:00:00
> * It should be easier to do the tests with "make tests". Users shouldn't
> need to modify the makefile; it should automatically do the 64-bit tests
> on all platforms, making sure that they do the right thing on 32-bit
> hosts. We can put the expensive tests in a separate target "make
> tests-expensive". Perhaps we should fold this into "make check" (with a
> new rule "make check-expensive") to simplify things further.
> * The snprintfs should be fixed to never do silent truncation on any
> platform. The second one can use INT_STRLEN_MAXIMUM to do that.
> * The signal handler has undefined behavior. It should just write (not
> printf) to stderr and then _exit (not exit).
> * Would you mind if we changed "if(" to "if (", and similarly for other
> keywords, for consistency with the other tz code?
More information about the tz