Subject: Re: [PATCH v2 1/3] t0006: add support for approxidate test date adjustment

From: Jeff King <[email-masked]>
Date: Tue, 12 May 2026 14:35:19 -0400

On Tue, May 12, 2026 at 05:54:28PM +0300, Tuomas Ahola wrote:

> t0006 uses a hard-coded test date and provides no convenient
> way to override it temporarily.  Add an optional parameter to
> check_approxidate to adjust the time as needed, and demonstrate
> the feature with a new test.

Makes sense, but two small-ish comments:

> +check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' success +48

One, it sucks to have to say "success" here, but is awkward because now
we have two optional arguments. There's nobody passing "failure" right
now, so we could just drop support, though that might be annoying later
when somebody wants to add a failing test. But we could perhaps switch
to allowing:

  check_approxidate --failure 'January 5th yesterday' ...etc

which is fairly natural.

This is something we've run into in many different test scripts, and I
think the harness could do a better job of supporting this. Perhaps with
something like:

  test_expect() {
	if "$GIT_TEST_EXPECT" = "fail"
	then
		test_expect_failure "$@"
	else
		test_expect_success "$@"
  }

  test_fails() {
	# probably needs to be more careful of one-shot with functions
	GIT_TEST_EXPECT=fail "$@"
  }

  # this is a normal passing test snippet
  test_expect 'foo' 'git foo'

  # this one expects failure, but it can be toggled easily by
  # removing the leading "test_fails" wrapper. Not much better
  # than swapping out s/failure/success/ now. But...
  test_fails test_expect 'bar' 'git bar'

  # this one just does the right thing if the helper function
  # is using test_expect under the hood
  test_fails check_approxidate ...

I dunno. It is probably too big a rabbit hole to do before your series,
so mostly I'm just thinking out loud.

> +check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' success +48

The second thing is that "+48" is pretty opaque. It's a relative offset
to some arbitrary point. To some degree the script already suffers from
that (all of the tests are using some arbitrary point), but I think the
offset (without units!) adds a layer of indirection that makes it even
more confusing.

I wonder how hard it would be to just take an arbitrary time instead,
and then you could write:

  check_approxidate 'January 5th yesterday' '2008-12-31 19:20:00' '2009-08-28 12:00:00'

or whatever. There is a chicken-and-egg problem with testing our date
routines and using the date routines to parse out the starting point.
But I think for approxidate, we could be using the strict parser (tested
separately earlier via check_parse) to handle the base time.

-Peff