Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime

From: Jeff King <peff@peff.net>
Date: Mar 26, 2014, 19:25 UTC
Message-ID: <20140326192536.GA13989@sigill.intra.peff.net>
In-Reply-To: <xmqqr45oixa6.fsf@gitster.dls.corp.google.com>

On Wed, Mar 26, 2014 at 12:18:25PM -0700, Junio C Hamano wrote:

Show 9 quoted lines

> > +		echo "$candidate" >expect &&
> > +		test_cmp expect actual &&
> > +		return 0
> > +	done
> > +	return 1
> > +}
> 
> It actually may be easier to understand if you write a trivial case
> statement at the sole calling site of this helper function, though.

Yeah, perhaps. I wanted to build on test_cmp because it has nice output for the failure case, but it is probably simple enough to do:

  output=$(cat actual)
  case "$output" in
  ...
  *) echo >&2 "unrecognized date: $output"

Show 9 quoted lines

> In any case, would it really be essential to make sure that the
> output shows a phony (or a seemingly real) datestamp for this test?
> 
> The primary thing you wanted to achieve by the "gmtime gave us NULL,
> let's substitute it with an arbitrary value to avoid dereferencing
> the NULL" change was *not* that we see that same arbitrary value
> comes out of the system, but that we do not die by attempting to
> reference the NULL, I think.  Not dying is the primary thing we want
> to (and we already do) test, no?

I think there are really two separate behaviors we are testing here (and in the surrounding tests):

  1. Don't segfault if gmtime returns NULL.

  2. Whenever we cannot process a date (either because gmtime fails, or
     because we fail before even getting the value to gmtime),
     consistently return the sentinel date (so the reader can easily
     know it's bogus).

Having the test be particular about its output helped us find a case where FreeBSD did not trigger (1), but did trigger (2), by returning a blanked "struct tm".

I'm open to the argument that (2) is not worth worrying about that much if it is a hassle to test. But I don't think it is that much hassle (yet, anyway).

-Peff