From: Jeff King <peff@peff.net>
To: Tuomas Ahola <taahol@utu.fi>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 1/3] t0006: add support for approxidate test date adjustment
Date: Tue, 12 May 2026 14:35:19 -0400 [thread overview]
Message-ID: <20260512183519.GA70851@coredump.intra.peff.net> (raw)
In-Reply-To: <20260512145430.13212-2-taahol@utu.fi>
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
next prev parent reply other threads:[~2026-05-12 18:35 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-18 18:01 [PATCH 0/2] approxidate: tweak special date formats Tuomas Ahola
2025-03-18 18:02 ` [PATCH 1/2] approxidate: make "specials" respect fixed day-of-month Tuomas Ahola
2025-04-04 8:19 ` Jeff King
2025-03-18 18:02 ` [PATCH 2/2] approxidate: overwrite tm_mday for `now` and `yesterday` Tuomas Ahola
2025-04-04 8:40 ` Jeff King
2026-05-12 14:54 ` [PATCH v2 0/3] approxidate: tweak special date formats Tuomas Ahola
2026-05-12 14:54 ` [PATCH v2 1/3] t0006: add support for approxidate test date adjustment Tuomas Ahola
2026-05-12 16:34 ` Junio C Hamano
2026-05-12 18:35 ` Jeff King [this message]
2026-05-12 14:54 ` [PATCH v2 2/3] approxidate: make "specials" respect fixed day-of-month Tuomas Ahola
2026-05-12 16:52 ` Junio C Hamano
2026-05-12 14:54 ` [PATCH v2 3/3] approxidate: use deferred mday adjustments for "specials" Tuomas Ahola
2026-05-14 11:55 ` [PATCH v3 0/4] approxidate: tweak special date formats Tuomas Ahola
2026-05-14 11:55 ` [PATCH v3 1/4] t0006: add support for approxidate test date adjustment Tuomas Ahola
2026-05-14 11:55 ` [PATCH v3 2/4] approxidate: alias "today" to "now" Tuomas Ahola
2026-05-14 15:36 ` Junio C Hamano
2026-05-14 21:07 ` Tuomas Ahola
2026-05-15 1:27 ` Junio C Hamano
2026-05-15 1:38 ` Junio C Hamano
2026-05-15 5:02 ` Tuomas Ahola
2026-05-14 11:55 ` [PATCH v3 3/4] approxidate: make "specials" respect fixed day-of-month Tuomas Ahola
2026-05-14 16:06 ` Junio C Hamano
2026-05-14 11:55 ` [PATCH v3 4/4] approxidate: use deferred mday adjustments for "specials" Tuomas Ahola
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260512183519.GA70851@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=taahol@utu.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.