Round-down years in "years+months" relative date view

Previously, a commit from 1 year and 7 months ago would display as "2 years, 7 months ago".

---

Here's my test script. Let me know if you'd rather have it as part of the test suite.

#!/bin/sh
set -e
REPO="git-relative-dates-test"
rm -rf "$REPO"
mkdir "$REPO"
cd "$REPO"
git init
NOW=date +%s
env GIT_AUTHOR_DATE=expr $NOW - \( 365 + 220 \) \* 24 \* 60 \* 60 git commit --allow-empty -m old-commit git log --date=relative

 date.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 1de1845..e848d96 100644
@@ -137,7 +137,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 		}
 		/* Give years and months for 5 years or so */
 		if (diff < 1825) {
-			unsigned long years = (diff + 183) / 365;
+			unsigned long years = diff / 365;
 			unsigned long months = (diff % 365 + 15) / 30;
 			int n;
 			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",

1.6.0.4

Wow, embarrassing.

I couldn't find any tests related to relative date processing, so it would be really nice to have some. But I'm not sure of the best way to do it without dealing with race conditions. Annoyingly, show_date calls gettimeofday at a pretty low level, so there isn't a way of instrumenting it short of LD_PRELOAD trickery (which is probably not very portable).

But maybe a patch like this is worth doing, which would allow us to test in a repeatable fashion:

---

index e848d96..db2f831 100644
@@ -86,6 +86,33 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+static int current_time(struct timeval *now)
+{
+	static struct timeval fake_time;
+	static int use_fake_time = -1;
+
+	if (use_fake_time == -1) {
+		const char *x = getenv("GIT_FAKE_TIME");
+		if (x) {
+			char buf[50];
+			if (parse_date(x, buf, sizeof(buf)) <= 0)
+				die("unable to parse GIT_FAKE_TIME");
+			fake_time.tv_sec = strtoul(buf, NULL, 10);
+			fake_time.tv_usec = 0;
+			use_fake_time = 1;
+		}
+		else
+			use_fake_time = 0;
+	}
+
+	if (use_fake_time == 1) {
+		memcpy(now, &fake_time, sizeof(*now));
+		return 0;
+	}
+
+	return gettimeofday(now, NULL);
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;

@@ -99,7 +126,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	if (mode == DATE_RELATIVE) {
 		unsigned long diff;
 		struct timeval now;
-		gettimeofday(&now, NULL);
+		current_time(&now);
 		if (now.tv_sec < time)
 			return "in the future";
 		diff = now.tv_sec - time;

@@ -929,7 +956,7 @@ unsigned long approxidate(const char *date)
 	if (parse_date(date, buffer, sizeof(buffer)) > 0)
 		return strtoul(buffer, NULL, 10);
 
-	gettimeofday(&tv, NULL);
+	current_time(&tv);
 	time_sec = tv.tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;

Maybe better prepare the _test_ so that it uses current time and time arithmetics then put yet another cludge in operational code? Especially when we already have a greate number of GIT_ environment variables, documented nowhere, with effects not immediately obvious:

  $ git grep -n '"GIT_'| perl -ne '/"(GIT_\w+)/ && print "$1\n"' |
sort |uniq | wc -l
  49

  $ git grep -n '"GIT_'|grep ^Documentation
  $

GIT_FLUSH? GIT_SEND_EMAIL_NOTTY?! GIT_CHERRY_PICK_HELP?!!

But that's the point: you can't do that without a race condition. Your test gets a sense of the current time, then runs git, which checks the current time again. How many seconds elapsed between the two checks?

I guess it is good enough for testing large time spans, but I was hoping for a comprehensive time test.

-Peff

How _many_ do you need?

I don't understand what you're trying to say. My point is that if you are checking results to a one-second precision, you need to know whether zero seconds elapsed, or one second, or two seconds, or whatever to get a consistent result.

-Peff

Taking this particular case as an example, can't we just set the time of the _commit_ back in time? We can.
And we don't need to know the difference precisely, it can be something like /[0-9]+ ago/, can't it?
Ok, it is possible, that something goes terribly wrong and the test suite freezes for an extended period of time, so the pattern above does not apply anymore. In this case, wont you prefer the test suite to break? Ok, maybe not, if the freeze was an Ctrl-Z pressed at unlucky moment. Which involves an operator online and looking, and action and reaction will be both visible.

So, yes, it is not absolutely safe, but this approach has no side effects on the working code. And very low probability of something go wrong.

To no-one in particular, Gitzilla mumbles "To do this right(tm) would probably require LD_PRELOAD magic with a environment variable invocation.

I agree with your concern. This is why I created the --index-version switch to pack-objects.

However I was hoping for a current time trickery solution that could live in test-date.c instead of interfering with the main code in such a way.

Did a quick test to override the library version:

index 62e8f23..0bcd0c9 100644
@@ -1,5 +1,10 @@
 #include "cache.h"
 
+int gettimeofday(struct timeval *tv, struct timezone *tz)
+{
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int i;

Result:

$ ./test-date now
now -> bad -> Wed Dec 31 19:00:00 1969
now -> Tue Jan 22 10:48:24 10199

So this seems to work. ;-)

Nicolas

Thanks, that is a much better solution. And I don't know offhand of any portability problems in overriding the library at link time.

-Peff

Microsoft's compiler and libraries? MacOSX?

Are you saying you know those to be platforms with problems, or are you asking whether those platforms will have problems?

-Peff

Both: MS never had weak/vague linkage, but I don't know about MacOSX. I suspect them to have the same deficiency, but I'd be glad to be wrong.

Hm, how about supplying show_date and approxidate with current time? Like this:

/* exported */
const char *show_date_rel(unsigned long time, int tz, struct timeval *now) {
... the DATE_RELATIVE path of show_date
}

const char *show_date(unsigned long time, int tz, enum date_mode mode) {
struct timeval now;

  if (mode == DATE_RELATIVE) {
    gettimeofday(&now, NULL);
    return show_date_rel(time, tz, &now);

}
... other paths of old show_date
}

Still affects the performance, but much less, and no side effects. And you can test show_date_rel path in test-date.c

This is not about weak or vague linkage. This is plain basic linker feature where no library object needs to be linked if there is no symbol to resolve.

Are you able to test it?

Nicolas

Maybe I missed something, but wasn't the idea to overwrite gettimeofday with a public gettimeofday, defined in one of the object files? And shouldn't a linker complain regarding duplicated symbols, unless the other (library) symbol is defined as a weak symbol, allowing overriding it with another symbol of stronger linkage?

Only the MS, but it is not interesting.

Yes, in test-date.o.

Normally a linker would search for new objects to link only when there are still symbols to resolve. If the library is well architected (mind you I don't know if that is the case on Windows or OS X) you should find many small object files in a library, so to have only related functions together in a single object for only the needed code to be linked in the final binary. Hence the printf symbol should be in a separate object file than gettimeofday, etc.

Only if the library's object file containing gettimeofday also contains another symbol pulled by the linker will you see a duplicated symbol error. But this is still a possibility. So your proposal is probably cleaner.

Nicolas

Nicolas Pitre, Fri, Aug 28, 2009 21:27:53 +0200:

Is it so for dynamic linking as well? Like in libc.so?

Yes. The linker still links against stubs in that case.

Nicolas

From: Alex Riesen <[email protected]>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code.
---
Nicolas Pitre, Fri, Aug 28, 2009 19:28:34 +0200:

This is what I mean with "supplying current time":

 cache.h |    2 +
 date.c  |  130 ++++++++++++++++++++++++++++++++++----------------------------
 2 files changed, 73 insertions(+), 59 deletions(-)

index dd7f71e..3fb0166 100644
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1

1.6.4.1.261.gf9874

Oh, bugger...

Exactly.

From fe67532bdf095dc9ebc0c7dd67be384e807a197c Mon Sep 17 00:00:00 2001
From: Alex Riesen <[email protected]>
Date: Fri, 28 Aug 2009 20:59:59 +0200
Subject: [PATCH] Add date formatting functions with current time explicitely formatted

It should allow safe testing of this part of the code.
---
Alex Riesen, Fri, Aug 28, 2009 21:20:50 +0200:

Rather like this, I mean :)

 cache.h |    2 +
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 89 insertions(+), 63 deletions(-)

index dd7f71e..3fb0166 100644
@@ -731,9 +731,11 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz, const struct timeval *now);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1

1.6.4.1.261.gf9874

To complement the testability of approxidate.
---
Alex Riesen, Fri, Aug 28, 2009 21:33:02 +0200:

And this should really allow testing of it:

    $ ./test-date '10.days.ago'
    10.days.ago -> bad -> Thu Jan  1 01:00:00 1970
    10.days.ago -> Tue Aug 18 22:50:20 2009

    relative: 10.days.ago -> Fri Dec 22 12:00:00 1989

    relative: 10 days ago, out of Fri Dec 22 12:00:00 1989

    $

According to Wikipedia, absolutely nothing of note happened at the day 10 January, 1990.

 test-date.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

index 62e8f23..dcc7973 100644
@@ -4,6 +4,17 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;

@@ -15,6 +26,12 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s\n", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s\n",
+		       show_date_relative(t, 0, &when), ctime(&t));
 	}
 	return 0;
 }

1.6.4.1.263.g468a

Alex Riesen, Fri, Aug 28, 2009 22:52:32 +0200:

1 Jan. 1990, not 10.

What are these blank lines for? Is this intended as a serious submission for inclusion? I am having a hrad time to guess, especially you did not sign this off, nor Cc'ed me.

ctime(3) artifact (it adds a \n in the output buffer), which I missed.

Not yet. AFAICS, test-date is never used in our test suite.

Right, that's because I'm not sure myself. Frankly, I'm not convinced we have to test every single thing. In my experience, the bigger a test suite, the less are people inclined to use it (including setting up automatic test runs).

Jeff, Nicolas? Is this test enough? Are there any other code paths you want to include in the test?

And sorry for having you missed in Cc:, that wasn't intended.

No, it isn't, but I think the point of this is to change that. So it is useless without an extra patch to the test suite. I'll try to put something together.

I think this is a useful addition to the test suite. The bug David fixed was obvious, but it sat for a year because of poor test coverage. Linus fixed several approxidate bugs recently. The approxidate code is notoriously temperamental, so it is a good thing to be checking for regressions.

And I don't think our test suite is nearly big enough to start worrying about getting people not to use it. Without CVS and SVN tests, I can run it on 3-year-old hardware in less than a minute. Either you bother to run it or not, but I doubt that adding one new test script is going to break the bank.

-Peff

Thanks :)

The main purpose is to allow predictable testing of the code.

---

Have show_date_relative supplied the output buffer. As it is a new interface, it can as well be a little bit more generic than its sole caller. test-date.c is updated and shall follow in a moment.

And, after a while thinking, I am convinced that Jeff has a point and used a more "internal" name for approxidate's recent "bottom half".

 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)

index dd7f71e..1586f33 100644
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1

1.6.4.1.294.g16262

To complement the testability of approxidate.

---

Alex Riesen, Sun, Aug 30, 2009 11:13:46 +0200:

here it goes.

 test-date.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

index 62e8f23..4cb146d 100644
@@ -4,6 +4,19 @@ int main(int argc, char **argv)
 {
 	int i;
 
+	/* see date.c, function show_date_relative */
+	char timebuf[(sizeof(long) * 5 / 2 + sizeof(" minutes ago,")) * 2];
+	struct tm tm;
+	struct timeval when = {0, 0};
+	tm.tm_sec  = 0;
+	tm.tm_min  = 0;
+	tm.tm_hour = 12;
+	tm.tm_mday = 1;
+	tm.tm_mon  = 0  /* January */;
+	tm.tm_year = 90 /* 1990 */ ;
+	tm.tm_isdst = -1;
+	when.tv_sec = mktime(&tm);
+
 	for (i = 1; i < argc; i++) {
 		char result[100];
 		time_t t;

@@ -15,6 +28,13 @@ int main(int argc, char **argv)
 
 		t = approxidate(argv[i]);
 		printf("%s -> %s\n", argv[i], ctime(&t));
+
+		t = approxidate_relative(argv[i], &when);
+		printf("relative: %s -> %s", argv[i], ctime(&t));
+
+		printf("relative: %s, out of %s",
+		       show_date_relative(t, 0, &when, timebuf,sizeof(timebuf)),
+		       ctime(&t));
 	}
 	return 0;
 }

1.6.4.1.294.g16262

FYI, I am munging test-date to match the test script I am writing, so don't bother with that patch.

-Peff

Here is what my patch is looking like. Please give any comments, and then I will resubmit in a form that will be simpler for Junio, which should be a series with:

- your patch to refactor date.c
- this patch (this version uses the original interface to show_relative; I will tweak to match the new patch you just sent)
- another patch to go on top of lt/approxidate to test recent fixes from Linus

---

new file mode 100755
index 0000000..4beb44b
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done

From: Alex Riesen <[email protected]>

The main purpose is to allow predictable testing of the code.

---
 cache.h |    5 ++
 date.c  |  152 +++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 94 insertions(+), 63 deletions(-)

index 808daba..5fad24c 100644
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1

1.6.4.2.375.g73938

Bleh. I just started working on a 4/3 that would test Linus' recent approxidate changes, but then I realized that this massive date.c refactoring conflicts with his changes.

I think the most sane thing is to rebase the whole series on top of lt/approxidate. Let me see what I can do.

-Peff

And here it is. It was a little more complex than a simple rebase because lt/approxidate actually introduced new bugs. :) Hopefully this will be the last re-roll required.

The new series applies on top of lt/approxidate, and contains:

[1/4]: Add date formatting and parsing functions relative to a given time [2/4]: refactor test-date interface
[3/4]: tests: add date printing and parsing tests
[4/4]: fix approxidate parsing of relative months and years

-Peff

From: Alex Riesen <[email protected]>

The main purpose is to allow predictable testing of the code.

---
Same as previous 1/3, but rebased onto lt/approxidate topic. The merge ended up as quite a mess because of textual differences, so I had to fix up a fair bit by hand; Alex, please confirm that I didn't screw anything up too badly right before putting your name at the top. ;)

 cache.h |    5 ++
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 92 insertions(+), 63 deletions(-)

index 808daba..5fad24c 100644
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1

1.6.4.2.373.g5881fd

Looks good.

The test-date program goes back to the early days of git, where it was presumably used to do manual sanity checks on changes to the date code. However, it is not actually used by the test suite to do any sort of automatic of systematic tests.

This patch refactors the interface to the program to try to make it more suitable for use by the test suite. There should be no fallouts to changing the interface since it is not actually installed and is not internally called by any other programs.

The changes are:

- add a "mode" parameter so the caller can specify which operation to test

- add a mode to test relative date output from show_date

- allow faking a fixed time via the TEST_DATE_NOW environment variable, which allows consistent automated testing

- drop the use of ctime for showing dates in favor of our internal iso8601 printing routines. The ctime output is somewhat redundant (because of the day-of-week) which makes writing test cases more annoying.

---
Same as earlier 2/3.

 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)

rewrite test-date.c (63%)

dissimilarity index 63%
index 62e8f23..5b0a220 100644
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}

1.6.4.2.373.g5881fd

Until now, there was no coverage of relative date printing or approxidate parsing routines (mainly because we had no way of faking the "now" time for relative date calculations, which made consistent testing impossible).

This new script tries to exercise the basic features of show_date and approxidate. Most of the tests are just "this obvious thing works" to prevent future regressions, with a few exceptions:

- We confirm the fix in 607a9e8 that relative year/month dates in the latter half of a year round correctly.

- We confirm that the improvements in b5373e9 and 1bddb25 work.

- A few tests are marked to expect failure, which are regressions recently introduced by the two commits above.

---
Similar to earlier 3/3, but improvements and regressions from lt/approxidate included.

Linus, when you posted the approxidate fixes earlier, you mentioned you might have some other corner cases. I think you were just referring to the stuff you improved in the followup patch, but if you know of more broken-ness, we should probably include it here.

 t/t0006-date.sh |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh

new file mode 100755
index 0000000..02cd565
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_${3:-success} "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_${3:-success} "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 bad
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_${3:-success} "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 19:20:00'
+check_approxidate '06.05.2009' '2009-05-06 19:20:00'
+
+check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
+check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
+check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
+
+test_done

1.6.4.2.373.g5881fd

Ugh. I just realized (when explaining how awesome git resurrect was in another mail) that I managed to bungle these commit hashes (and the one mentioned in the following patch).

What happened is that I was building on the topic branch and lazily did a "rebase -i origin" to fix up my patches. I left the first two patches untouched, of course, but they still ended up with new committer information.

As my patches are merged to 'next' already, I think it is too late to fixup the commit message. But for posterity, the correct referenced commits are 9029055 and 36e4986.

Caveat rebaser.

-Peff

These were broken by b5373e9. The problem is that the code marks the month and year with "-1" for "we don't know it yet", but the month and year code paths were not adjusted to fill in the current time before doing their calculations (whereas other units follow a different code path and are fine).

---
This one is new from the last series, as it fixes bugs only found in lt/approxidate.

 date.c          |    5 ++++-
 t/t0006-date.sh |    4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

index 8e57e5e..e9ee4aa 100644
@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "months") >= 5) {
-		int n = tm->tm_mon - *num;
+		int n;
+		update_tm(tm, now, 0); /* fill in date fields if needed */
+		n = tm->tm_mon - *num;
 		*num = 0;
 		while (n < 0) {
 			n += 12;

@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "years") >= 4) {
+		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
 		return end;

index 02cd565..a4d8fa8 100755
@@ -53,8 +53,8 @@ check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
 check_approxidate yesterday '2009-08-29 19:20:00'
 check_approxidate 3.days.ago '2009-08-27 19:20:00'
 check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
-check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
-check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
 
 check_approxidate '6am yesterday' '2009-08-29 06:00:00'
 check_approxidate '6pm yesterday' '2009-08-29 18:00:00'

1.6.4.2.373.g5881fd

The test-date program goes back to the early days of git, where it was presumably used to do manual sanity checks on changes to the date code. However, it is not actually used by the test suite to do any sort of automatic of systematic tests.

This patch refactors the interface to the program to try to make it more suitable for use by the test suite. There should be no fallouts to changing the interface since it is not actually installed and is not internally called by any other programs.

The changes are:

- add a "mode" parameter so the caller can specify which operation to test

- add a mode to test relative date output from show_date

- allow faking a fixed time via the TEST_DATE_NOW environment variable, which allows consistent automated testing

- drop the use of ctime for showing dates in favor of our internal iso8601 printing routines. The ctime output is somewhat redundant (because of the day-of-week) which makes writing test cases more annoying.

---
I mulled over replacing ctime for a bit, as we are testing git's date code with other parts of git's date code. But it really is more convenient for writing test cases to use iso8601, since you don't have to calculate the day-of-week (and I also think it is a bit more readable). And our iso8601 code is dead simple, so I am not too worried about a bug in it hiding a bug elsewhere.

 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)

rewrite test-date.c (63%)

dissimilarity index 63%
index 62e8f23..5b0a220 100644
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}

1.6.4.2.375.g73938

Until now, there was no coverage of relative date printing or approxidate parsing routines (mainly because we had no way of faking the "now" time for relative date calculations, which made consistent testing impossible).

This new script tries to exercise the basic features of show_date and approxidate. The only specific problem case tested is showing relative year/month dates in the latter half of a year, as fixed by 607a9e8.

---
Like I said, this is really just to exercise the basic code paths. But now that the infrastructure is there, we can add any corner cases or verify new features or bug fixes as they come up. Patches welcome. :)

 t/t0006-date.sh |   71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh

new file mode 100755
index 0000000..4beb44b
@@ -0,0 +1,71 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_success "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_success "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 '2008-02-14 00:00:00 +0000'
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_success "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 00:00:00'
+check_approxidate '06.05.2009' '2009-05-06 00:00:00'
+
+test_done

1.6.4.2.375.g73938

Yes, I think this is the only comment I can make.

The world changed since 1980 :) There is already three things happened at the day (http://en.wikipedia.org/wiki/August_2009),
and it is not evening yet (well, here in Europe)

This part is about checking what show_date produces (the first number is an offset from now in seconds, and the second is what we expect), so it always has spaces.

See the check_approxidate section further down for an example of parsing what you are talking about.

-Peff

Ah, I see