Re: [PATCH 3/3] t0006-date.sh: add `human` date format tests.

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 2, 2019, 18:15 UTC
Message-ID: <xmqqva37j595.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181231003150.8031-4-ischis2@cox.net>

"Stephen P. Smith" <ischis2@cox.net> writes:

Show 6 quoted lines

> +# Subtract some known constant time and look for expected field format
> +TODAY_REGEX='5 hours ago'
> +THIS_YEAR_REGEX='[A-Z][a-z][a-z] [A-Z][a-z][a-z] [0-9]* [012][0-9]:[0-6][0-9]'
> +MORE_THAN_A_YEAR_REGEX='[A-Z][a-z][a-z] [A-Z][a-z][a-z] [0-9]* [0-9][0-9][0-9][0-9]'
> +check_human_date "$(($(date +%s)-18000)) +0200" $TODAY_REGEX # 5 hours ago
> +check_human_date "$(($(date +%s)-432000)) +0200" $THIS_YEAR_REGEX  # 5 days ago

'date +%s' is used everywhere in this patch but has never been used in our test suite before. It is not portable.

We perhaps can use "test-tool date timestamp", like so

	check_human_date $(test-tool date timestamp "18000 seconds ago") ...

or moving the part that munges 18000 into the above form inside check_human_date helper function, e.g.

	check_human_date () {
		commit_date=$(test-tool date timestamp "$1 seconds ago")
		commit_date="$commit_date +0200"
                expect=$2
		...
	}

which would let us write

	check_human_date 432000" $THIS_YEAR_REGEX  # 5 days ago

Show 10 quoted lines

> +check_human_date() {
> +	commit_date=$1
> +	expect=$2
> +	test_expect_success "$commit_date" "
> +		echo $expect $commit_date >dates && 
> +		git add dates &&
> +		git commit -m 'Expect String' --date=\"$commit_date\" dates &&
> +		git log -1 --date=human | grep \"^Date:\" >actual &&
> +		grep \"$expect\" actual
> +"

As the body of the test_expect_success helper is eval'ed, variables $commit_date and $expect should be visible to it, without turning them into values before executing test_expect_success function, i.e.

	test_expect_success "$commit_date" '
		echo "$expect $commit_date" >dates &&
		...
		git commit -m "Expect String" --date="$commit_date" dates &&
		git show -s --date=human | grep '^Date:" >actual &&
		grep "$expect" actual
	'

which would reduce the need for unreadable backslashes.

Instead of duplicating, perhaps move this to a more common place? Would it make sense to make it "check_date_format ()" helper by passing another argument to parameterize --date=human part

Show 8 quoted lines

> +check_human_date() {
> +	commit_date=$1
> +	expect=$2
> +	test_expect_success "$commit_date" "
> +		echo $expect $commit_date >dates && 
> +		git add dates &&
> +		git commit -m 'Expect String' --date=\"$commit_date\" dates &&
> +		git show --date=human | grep \"^Date:\" >actual &&

Using "show" here is much better than "log -1" above; using "show -s" would be even better.

Show 18 quoted lines

> +		grep \"$expect\" actual
> +"
> +}
> +
> +TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
> +THIS_YEAR_REGEX='[A-Z][a-z][a-z] [A-Z][a-z][a-z] [0-9]* [012][0-9]:[0-6][0-9]'
> +MORE_THAN_A_YEAR_REGEX='[A-Z][a-z][a-z] [A-Z][a-z][a-z] [0-9]* [0-9][0-9][0-9][0-9]'
> +check_human_date "$(($(date +%s)-18000)) +0200" $TODAY_REGEX # 5 hours ago
> +check_human_date "$(($(date +%s)-432000)) +0200" $THIS_YEAR_REGEX  # 5 days ago
> +check_human_date "$(($(date +%s)-1728000)) +0200" $THIS_YEAR_REGEX # 3 weeks ago
> +check_human_date "$(($(date +%s)-13000000)) +0200" $THIS_YEAR_REGEX # 5 months ago
> +check_human_date "$(($(date +%s)-31449600)) +0200" $THIS_YEAR_REGEX # 12 months ago
> +check_human_date "$(($(date +%s)-37500000)) +0200" $MORE_THAN_A_YEAR_REGEX # 1 year, 2 months ago
> +check_human_date "$(($(date +%s)-55188000)) +0200" $MORE_THAN_A_YEAR_REGEX # 1 year, 9 months ago
> +check_human_date "$(($(date +%s)-630000000)) +0200" $MORE_THAN_A_YEAR_REGEX # 20 years ago
> +
> +
>  test_done