[3/3] t0006-date.sh: add `human` date format tests.
diff mbox series

Message ID 20181231003150.8031-4-ischis2@cox.net
State New
Headers show
Series
  • Add 'human' date format
Related show

Commit Message

Stephen P. Smith Dec. 31, 2018, 12:31 a.m. UTC
The `human` date format varies based on two inputs: the date in the
reference time which is constant and the local computers date which
varies.  Using hardcoded test expected output dates would require
holding the local machines date and time constant which is not
desireable.

Alternatively, letting the local date vary, which is the normal
situation, implies that the tests would be checking for formating
changes based on on a ref date relative to the local computers time.

When using `human` several fields are suppressed depending on the time
difference between the reference date and the local computer date. In
cases where the difference is less than a year, the year field is
supppressed. If the time is less than a day; the month and year is
suppressed.

Test using a regular expression to verify that fields that are
expected to be suppressed are not displayed.

Signed-off-by: Stephen P. Smith <ischis2@cox.net>
---
 t/t0006-date.sh | 24 ++++++++++++++++++++++++
 t/t4202-log.sh  | 24 ++++++++++++++++++++++++
 t/t7007-show.sh | 25 +++++++++++++++++++++++++
 3 files changed, 73 insertions(+)

Comments

Junio C Hamano Jan. 2, 2019, 6:15 p.m. UTC | #1
"Stephen P. Smith" <ischis2@cox.net> writes:

> +# 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

> +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

> +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.

> +		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
Stephen P. Smith Jan. 3, 2019, 2:36 a.m. UTC | #2
On Wednesday, January 2, 2019 11:15:02 AM MST Junio C Hamano wrote:
> 'date +%s' is used everywhere in this patch but has never been used
> in our test suite before.  It is not portable.
So I don't make this mistake again, Is there a reference somewhere for that is 
and 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
Thanks

>
> > +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.
I was worried about embedded spaces that might not be parsed correctly by the 
called function.  I will update

> 
> 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
I had considered that, but then noted that for the other formats specific 
strings were being used.  The use of specific strings was possible since the 
other formats were always guarenteed to have the same string literal due to a 
singe unvarying input.

I don't mind parameterize the format and it would make the solution more 
general.


> > "Stephen P. Smith" <ischis2@cox.net> writes:
> > +# 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
> > +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.

I was attempting to test both git log and git show.  For get log the `-1` was 
to only get the latest commit.

Are you suggesting that t4202-log.sh not be updated and that only and  t7007-
show.sh and t0006-date.sh updated?  

Side note:  I found when updating that all three scripts that log and show 
returned the same formats, but date returned a different string if the delta 
date was less than 24hours

I just noted that the patch 3/3 should be re-titled since the tests are 
currently for three commands.

Hope you are better.
sps
Junio C Hamano Jan. 3, 2019, 6:42 a.m. UTC | #3
Stephen & Linda Smith <ischis2@cox.net> writes:

> On Wednesday, January 2, 2019 11:15:02 AM MST Junio C Hamano wrote:
>> 'date +%s' is used everywhere in this patch but has never been used
>> in our test suite before.  It is not portable.
> So I don't make this mistake again, Is there a reference somewhere for that is 
> and is not portable?

I usually go to http://pubs.opengroup.org/onlinepubs/9699919799/

Even though we do not say "We'll use anything that is in POSIX.1; it
is your problem if your platform does not support it", we tend to
say "It's not even in POSIX, so let's see if we can avoid it".

>> Using "show" here is much better than "log -1" above; using "show
>> -s" would be even better.
>
> I was attempting to test both git log and git show.  For get log the `-1` was 
> to only get the latest commit.
>
> Are you suggesting that t4202-log.sh not be updated and that only and  t7007-
> show.sh and t0006-date.sh updated?  

I am saying that using "log -1" and "show" in different tests _only_
for the value of "Date:" field does not buy us much.  And by unifying,
I was hoping that the single helper can be placed in a common file
that is dot-sourced by these three scripts more easily.

Thanks.
Jeff King Jan. 3, 2019, 7:44 a.m. UTC | #4
On Sun, Dec 30, 2018 at 05:31:50PM -0700, Stephen P. Smith wrote:

> The `human` date format varies based on two inputs: the date in the
> reference time which is constant and the local computers date which
> varies.  Using hardcoded test expected output dates would require
> holding the local machines date and time constant which is not
> desireable.
> 
> Alternatively, letting the local date vary, which is the normal
> situation, implies that the tests would be checking for formating
> changes based on on a ref date relative to the local computers time.

We already have $TEST_DATE_NOW, which "test-tool date" will respect for
various commands to pretend that it's currently a particular time. I
think you'd need to add a sub-command similar to "relative" (which
directly calls show_date_relative()) which calls into the "human" code.

Note that there _isn't_ a way to have actual non-test git programs read
the current time from an environment variable (as opposed to actually
calling gettimeofday()).

-Peff
Stephen P. Smith Jan. 3, 2019, 1:12 p.m. UTC | #5
On Thursday, January 3, 2019 12:44:22 AM MST Jeff King wrote:
> We already have $TEST_DATE_NOW, which "test-tool date" will respect for
> various commands to pretend that it's currently a particular time. I
> think you'd need to add a sub-command similar to "relative" (which
> directly calls show_date_relative()) which calls into the "human" code.

I'll investigate.  Looks like this comment is related other comments.

> Note that there _isn't_ a way to have actual non-test git programs read
> the current time from an environment variable (as opposed to actually
> calling gettimeofday()).
Agreed

> 
> -Peff
Stephen P. Smith Jan. 3, 2019, 1:20 p.m. UTC | #6
On Wednesday, January 2, 2019 11:42:20 PM MST Junio C Hamano wrote:
> > Are you suggesting that t4202-log.sh not be updated and that only and 
> > t7007- show.sh and t0006-date.sh updated?
> 
> I am saying that using "log -1" and "show" in different tests _only_
> for the value of "Date:" field does not buy us much.  And by unifying,
> I was hoping that the single helper can be placed in a common file
> that is dot-sourced by these three scripts more easily.

Thanks for the clarification.
Philip Oakley Jan. 3, 2019, 9:14 p.m. UTC | #7
On 02/01/2019 18:15, Junio C Hamano wrote:
> 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


Just a quick bikeshed: if used, would this have a year end 5 day 
roll-over error potential, or will it always use the single date?

(I appreciate it is just suggestion code, not tested)
Junio C Hamano Jan. 3, 2019, 9:45 p.m. UTC | #8
Philip Oakley <philipoakley@iee.org> writes:

> On 02/01/2019 18:15, Junio C Hamano wrote:
>> 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
>
>
> Just a quick bikeshed: if used, would this have a year end 5 day
> roll-over error potential, or will it always use the single date?

Hmph, interesting point.  Indeed, date.c::show_date_normal() decides
to hide the year portion if the timestamp and the current time share
the same year, so on Thu Jan 3rd, an attempt to show a commit made
on Mon Dec 31st of the same week would end up showing the year, so
yes, I agree with you that the above would break.

+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]'

>
> (I appreciate it is just suggestion code, not tested)
Stephen P. Smith Jan. 3, 2019, 11:57 p.m. UTC | #9
On Thursday, January 3, 2019 2:45:39 PM MST Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.org> writes:
> >> 
> >> 	check_human_date 432000 "$THIS_YEAR_REGEX" # 5 days ago
> > 
> > Just a quick bikeshed: if used, would this have a year end 5 day
> > roll-over error potential, or will it always use the single date?
> 
> Hmph, interesting point.  Indeed, date.c::show_date_normal() decides
> to hide the year portion if the timestamp and the current time share
> the same year, so on Thu Jan 3rd, an attempt to show a commit made
> on Mon Dec 31st of the same week would end up showing the year, so
> yes, I agree with you that the above would break.
> 

Thanks Philip.

I wrote the test just before the new year so didn't see the rollover.   I 
haven't run the test this year.
sps
Johannes Sixt Jan. 8, 2019, 9:27 p.m. UTC | #10
Am 31.12.18 um 01:31 schrieb Stephen P. Smith:
> +check_human_date () {
> +	time=$1
> +	expect=$2
> +	test_expect_success "check date ($format:$time)" '
> +		echo "$time -> $expect" >expect &&
> +		TZ=${zone:-$TZ} test-tool date show:"$format" "$time" >actual &&
> +		grep "$expect" actual
> +	'
> +}
> +
>   # arbitrary but sensible time for examples
>   TIME='1466000000 +0200'
>   check_show iso8601 "$TIME" '2016-06-15 16:13:20 +0200'
> @@ -52,6 +62,20 @@ check_show unix "$TIME" '1466000000'
>   check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
>   check_show raw-local "$TIME" '1466000000 +0000'
>   check_show unix-local "$TIME" '1466000000'
> +check_show human "$TIME" 'Jun 15 2016'
> +
> +# 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
> +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
>   
>   check_show 'format:%z' "$TIME" '+0200'
>   check_show 'format-local:%z' "$TIME" '+0000'
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 819c24d10e..d7f3b73650 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1707,4 +1707,28 @@ test_expect_success '--exclude-promisor-objects does not BUG-crash' '
>   	test_must_fail git log --exclude-promisor-objects source-a
>   '
>   
> +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
> +"
> +}
> +
> +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
> diff --git a/t/t7007-show.sh b/t/t7007-show.sh
> index 42d3db6246..0a0334a8b5 100755
> --- a/t/t7007-show.sh
> +++ b/t/t7007-show.sh
> @@ -128,4 +128,29 @@ test_expect_success 'show --graph is forbidden' '
>     test_must_fail git show --graph HEAD
>   '
>   
> +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 &&
> +		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

The $...REGEX expansions must be put in double-quotes to protect them 
from field splitting. But then the tests do not pass anymore (I tested 
only t4202). Please revisit this change.

-- Hannes
Stephen P. Smith Jan. 9, 2019, 12:44 a.m. UTC | #11
On Tuesday, January 8, 2019 2:27:22 PM MST Johannes Sixt wrote:
> Am 31.12.18 um 01:31 schrieb Stephen P. Smith:
> > +
> > +TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
<snip>
> The $...REGEX expansions must be put in double-quotes to protect them
> from field splitting. But then the tests do not pass anymore (I tested
> only t4202). Please revisit this change.
> 
> -- Hannes

I will later figure out why you are seeing the fields splitting but I am not.   
In the mean time I will change the quoting.

I started working on test updates based on prior comments this past weekend.

sps
Johannes Sixt Jan. 9, 2019, 6:58 a.m. UTC | #12
Am 09.01.19 um 01:44 schrieb Stephen P. Smith:
> On Tuesday, January 8, 2019 2:27:22 PM MST Johannes Sixt wrote:
>> Am 31.12.18 um 01:31 schrieb Stephen P. Smith:
>>> +
>>> +TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'
> <snip>
>> The $...REGEX expansions must be put in double-quotes to protect them
>> from field splitting. But then the tests do not pass anymore (I tested
>> only t4202). Please revisit this change.
> 
> I will later figure out why you are seeing the fields splitting but I am not.
> In the mean time I will change the quoting.

In this line

TODAY_REGEX='[A-Z][a-z][a-z] [012][0-9]:[0-6][0-9] .0200'

no field splitting occurs. The quoting is fine here. But notice that the 
value of $TODAY_REGEX contains blanks.

In this line

check_human_date "$(($(date +%s)-18000)) +0200" $TODAY_REGEX

the value of $TODAY_REGEX is substituted and then the value is split 
into fields at the blanks because the expansion is not quoted.

As a consequence, function check_human_date considers only the first 
part of $TODAY_REGEX, i.e. 'A-Z][a-z][a-z]' (which is parameter $2), but 
ignores everything else (because it does not use $3 or $4).

-- Hannes
Stephen P. Smith Jan. 10, 2019, 1:50 a.m. UTC | #13
On Tuesday, January 8, 2019 11:58:29 PM MST Johannes Sixt wrote:
> But notice that the value of $TODAY_REGEX contains blanks.
> 
> In this line
> 
> check_human_date "$(($(date +%s)-18000)) +0200" $TODAY_REGEX
> 
> the value of $TODAY_REGEX is substituted and then the value is split
> into fields at the blanks because the expansion is not quoted.
> 
> As a consequence, function check_human_date considers only the first
> part of $TODAY_REGEX, i.e. 'A-Z][a-z][a-z]' (which is parameter $2), but
> ignores everything else (because it does not use $3 or $4).
> 
> -- Hannes

I hadn't understood your original comment, but now i understand.   Will fix.

Patch
diff mbox series

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index ffb2975e48..f208a80867 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -40,6 +40,16 @@  check_show () {
 	'
 }
 
+check_human_date () {
+	time=$1
+	expect=$2
+	test_expect_success "check date ($format:$time)" '
+		echo "$time -> $expect" >expect &&
+		TZ=${zone:-$TZ} test-tool date show:"$format" "$time" >actual &&
+		grep "$expect" actual 
+	'
+}
+
 # arbitrary but sensible time for examples
 TIME='1466000000 +0200'
 check_show iso8601 "$TIME" '2016-06-15 16:13:20 +0200'
@@ -52,6 +62,20 @@  check_show unix "$TIME" '1466000000'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
 check_show raw-local "$TIME" '1466000000 +0000'
 check_show unix-local "$TIME" '1466000000'
+check_show human "$TIME" 'Jun 15 2016'
+
+# 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
+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
 
 check_show 'format:%z' "$TIME" '+0200'
 check_show 'format-local:%z' "$TIME" '+0000'
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 819c24d10e..d7f3b73650 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1707,4 +1707,28 @@  test_expect_success '--exclude-promisor-objects does not BUG-crash' '
 	test_must_fail git log --exclude-promisor-objects source-a
 '
 
+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
+"
+}
+
+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
diff --git a/t/t7007-show.sh b/t/t7007-show.sh
index 42d3db6246..0a0334a8b5 100755
--- a/t/t7007-show.sh
+++ b/t/t7007-show.sh
@@ -128,4 +128,29 @@  test_expect_success 'show --graph is forbidden' '
   test_must_fail git show --graph HEAD
 '
 
+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 &&
+		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