--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id DF0EE431FB6\r
+ for <notmuch@notmuchmail.org>; Wed, 3 Oct 2012 13:32:23 -0700 (PDT)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id vBCmkDSndEtk for <notmuch@notmuchmail.org>;\r
+ Wed, 3 Oct 2012 13:32:22 -0700 (PDT)\r
+Received: from mail-la0-f53.google.com (mail-la0-f53.google.com\r
+ [209.85.215.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 34DC5431FAE\r
+ for <notmuch@notmuchmail.org>; Wed, 3 Oct 2012 13:32:22 -0700 (PDT)\r
+Received: by lahl5 with SMTP id l5so3745573lah.26\r
+ for <notmuch@notmuchmail.org>; Wed, 03 Oct 2012 13:32:20 -0700 (PDT)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+ d=google.com; s=20120113;\r
+ h=from:to:cc:subject:in-reply-to:references:user-agent:date\r
+ :message-id:mime-version:content-type:x-gm-message-state;\r
+ bh=7kuxDjMOydSS+WAnh7FkBvjru2hZcUGcz73BHGWdpn4=;\r
+ b=JVT8v4KJFDYYbssh/DsJjqQdpcD8b/ucyLlC8vLH264unZk4+nALpCoJ3HBz3x0qtI\r
+ aD2es4LfTmZP/ZPEoz35X2P5M0ZClOpiR2c0H+ZGnHUf6lwD7cY/twumuDNHOHZ99aFX\r
+ qhyprID2zDq1WcPREPtsn3/HVixxvXT3AXJXIgvLtFtjy3gyEigxnPZBgPuD7jTUW4k1\r
+ j2BB1SIbwq8KiMg08OF2xpG3r5KnrbJOsQQq9KbE1ndbFQZZUwmiY462qQafn0q3ILp/\r
+ Wk2cgZfbD2YsjQw6NRNphe9h2hWNhh2eA0sDg19U0IbsAIf2WCh9SJ+tPvjfgq54jVH6\r
+ +RWQ==\r
+Received: by 10.152.144.2 with SMTP id si2mr2572296lab.26.1349296340620;\r
+ Wed, 03 Oct 2012 13:32:20 -0700 (PDT)\r
+Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi.\r
+ [80.223.81.27])\r
+ by mx.google.com with ESMTPS id xw14sm1649379lab.15.2012.10.03.13.32.18\r
+ (version=SSLv3 cipher=OTHER); Wed, 03 Oct 2012 13:32:19 -0700 (PDT)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org,\r
+ David Bremner <david@tethera.net>\r
+Subject: Re: [PATCH] test: Improve tests for the date/time parser module\r
+In-Reply-To: <87zk4e1f5k.fsf@steelpick.2x.cz>\r
+References: <cover.1347484177.git.jani@nikula.org>\r
+ <24186aafbdcb967b8f66c2390c928f3788ab6cbf.1347484177.git.jani@nikula.org>\r
+ <87zk4e1f5k.fsf@steelpick.2x.cz>\r
+User-Agent: Notmuch/0.14+34~g2c0277c (http://notmuchmail.org) Emacs/23.3.1\r
+ (i686-pc-linux-gnu)\r
+Date: Wed, 03 Oct 2012 23:32:16 +0300\r
+Message-ID: <87lifn47qn.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+X-Gm-Message-State:\r
+ ALoCoQl0iVlBwmLJfZf2vY7ACP1yrUNqNXjUyqlgf5peAuEPlVouO/EAaa1bRS75nZdz7AaNM9jz\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Wed, 03 Oct 2012 20:32:24 -0000\r
+\r
+On Tue, 25 Sep 2012, Michal Sojka <sojkam1@fel.cvut.cz> wrote:\r
+> This patch reworks date/time parser library test program to make it\r
+> easier to to write the actual tests. It also modifies the notmuch test\r
+> script and adds several new tests to it.\r
+\r
+Cool!\r
+\r
+> The INPUT file for the test contains both the dates to be parsed as well\r
+> as the "expected" results. The test program outputs the results in the\r
+> same format and replaces expected results with real results. Currently,\r
+> the "expected" results in the INPUT file correspond to the real results,\r
+> so the test passes. Some results are, however, different from what I\r
+> would expect - this is mentioned in the comments after '#'.\r
+\r
+Please see comments inline next to tests.\r
+\r
+>\r
+> This patch applies on top of Jani's patchset.\r
+> ---\r
+> It can be seen that there are several errors and unexpected results.\r
+> As I've already written, I'm not sure that the approach taken by this\r
+> library is the right one. I tend to agree with mina86, that using a\r
+> more systematic approach (such as bison) would be beneficial.\r
+\r
+Then we just have to agree to disagree here. :)\r
+\r
+> This is however not to say to throw this patchset away. Either Jani\r
+> will be able to fix all the corner cases. Or we can work together to\r
+> develop a better solution - add support for ranges to the bison\r
+> parser.\r
+\r
+I think I've got the cases pretty much covered, and they're mostly not\r
+about syntax and parsing, but rather semantics; what to do with the\r
+parsed data.\r
+\r
+> -Michal\r
+>\r
+> diff --git a/test/Makefile.local b/test/Makefile.local\r
+> index 9ae130a..b9105c7 100644\r
+> --- a/test/Makefile.local\r
+> +++ b/test/Makefile.local\r
+> @@ -20,7 +20,7 @@ $(dir)/symbol-test: $(dir)/symbol-test.o\r
+> $(call quiet,CXX) $^ -o $@ -Llib -lnotmuch $(XAPIAN_LDFLAGS)\r
+> \r
+> $(dir)/parse-time: $(dir)/parse-time.o parse-time-string/parse-time-string.o\r
+> - $(call quiet,CC) $^ -o $@\r
+> + $(call quiet,CC) $^ -o $@ -lrt\r
+> \r
+> .PHONY: test check\r
+> \r
+> diff --git a/test/parse-time-string b/test/parse-time-string\r
+> index 34b80d7..265437c 100755\r
+> --- a/test/parse-time-string\r
+> +++ b/test/parse-time-string\r
+> @@ -14,13 +14,48 @@ _parse_time ()\r
+> ${TEST_DIRECTORY}/parse-time --format=%s "$*"\r
+> }\r
+> \r
+> -test_begin_subtest "date(1) default format without TZ code"\r
+> -test_expect_equal "$(_parse_time Fri Aug 3 23:06:06 2012)" "$(_date Fri Aug 3 23:06:06 2012)"\r
+> +test_begin_subtest "Date parser tests"\r
+> +cat <<EOF > INPUT\r
+> +now -> Tue Jan 11 11:11:00 +0000 2011\r
+> +2010-1-1 -> parse_time_string() error: 5\r
+\r
+I think that's invalid per ISO 8601.\r
+\r
+> +Jan 2 -> Sat Jan 02 11:11:00 +0000 2010 # Why 2010?\r
+\r
+This is an interesting bug. The idea was that specifying a month without\r
+year would always refer to past. So when you give Jan 2011 in the\r
+reference time, Jan refers to the previous year. Seems simple, but\r
+looking closer, also "Jan 2 this year" would end up in 2010. Not good.\r
+\r
+It would probably be possible to fix this, but per the simplicity of\r
+implementation and unambiguity goals, I think I'll just make them refer\r
+to current year, at least for now. This may mean having to look into\r
+supporting "last {monthname,weekday}" for better usability, but I'll\r
+leave that as a future improvement.\r
+\r
+> +Mon -> Mon Jan 10 11:11:00 +0000 2011\r
+> +last Friday -> parse_time_string() error: 4\r
+\r
+"last <weekday>" is not supported.\r
+\r
+> +2 hours ago -> parse_time_string() error: 1\r
+\r
+"ago" is not supported.\r
+\r
+> +last month -> Sat Dec 11 11:11:00 +0000 2010\r
+> +month ago -> parse_time_string() error: 1\r
+\r
+Ditto.\r
+\r
+> +8am -> Tue Jan 11 08:00:00 +0000 2011\r
+> +9:15 -> Tue Jan 11 09:15:00 +0000 2011\r
+> +12:34 -> Tue Jan 11 12:34:00 +0000 2011\r
+> +monday -> Mon Jan 10 11:11:00 +0000 2011\r
+> +yesterday -> Mon Jan 10 11:11:00 +0000 2011\r
+> +tomorrow -> parse_time_string() error: 1\r
+\r
+"tomorrow" is not supported (do you get a lot of mail from the future?\r
+;)\r
+\r
+> + -> Tue Jan 11 11:11:00 +0000 2011 # Shouldn't empty string return an error???\r
+\r
+*shrug* It just starts with the reference time, and finds nothing to add\r
+or remove. Let's call it a feature.\r
+\r
+> \r
+> -test_begin_subtest "date(1) --rfc-2822 format"\r
+> -test_expect_equal "$(_parse_time Fri, 03 Aug 2012 23:07:46 +0100)" "$(_date Fri, 03 Aug 2012 23:07:46 +0100)"\r
+> +Aug 3 23:06:06 2012 -> Fri Aug 03 23:06:06 +0000 2012 # date(1) default format without TZ code\r
+> +Fri, 03 Aug 2012 23:07:46 +0100 -> Fri Aug 03 22:07:46 +0000 2012 # rfc-2822\r
+> +2012-08-03 23:09:37+03:00 -> Fri Aug 03 20:09:37 +0000 2012 # rfc-3339 seconds\r
+> \r
+> -test_begin_subtest "date(1) --rfc=3339=seconds format"\r
+> -test_expect_equal "$(_parse_time 2012-08-03 23:09:37+03:00)" "$(_date 2012-08-03 23:09:37+03:00)"\r
+> +10s -> Tue Jan 11 11:10:50 +0000 2011\r
+> +19701223s -> Wed Dec 23 11:10:59 +0000 1970 # Surprising - number is parsed as date and 's' as '1 second'\r
+\r
+Will be fixed.\r
+\r
+> +19701223 -> Wed Dec 23 11:11:00 +0000 1970\r
+> +\r
+> +19701223 +0100 -> Wed Dec 23 11:11:00 +0000 1970 # Timezone is ignored without an error\r
+\r
+It's not ignored. Date is specified, but the time comes from the\r
+reference time. It's the same absolute time regardless of the timezone.\r
+\r
+> +\r
+> +today ^-> Wed Jan 12 00:00:00 +0000 2011 # This should be 11 23:59:59\r
+\r
+See my previous mail. Can be fixed.\r
+\r
+> +today v-> Tue Jan 11 00:00:00 +0000 2011\r
+> +\r
+> +thisweek ^-> Sun Jan 16 00:00:00 +0000 2011 # This should be Sunday 23:59:59\r
+> +thisweek v-> Sun Jan 09 00:00:00 +0000 2011 # This should be Monday 00:00:00\r
+\r
+Implementation simplicity and dodging a localization issue. Start of the\r
+week is based on the tm_mday field of struct tm, where 0 == Sunday. Even\r
+if I personally agree Monday is the 1st day of the week.\r
+\r
+> +\r
+> +two months ago-> parse_time_string() error: 1 # Comments in the code suggest that this is supported\r
+\r
+When in doubt, trust code over comments. ;)\r
+\r
+> +two months -> Thu Nov 11 11:11:00 +0000 2010\r
+> +\r
+> +1348569850 -> parse_time_string() error: 4 # Seconds since epoch not yet supported? Backward compatibility in notmuch???\r
+> +10 -> parse_time_string() error: 4 # Seconds since epoch?\r
+\r
+Indeed, seconds since epoch not yet supported. There is no backwards\r
+compatibility issue, as you can still use the\r
+<initial-timestamp>..<final-timestamp> format as described in\r
+notmuch-search-terms(7) man page. The new date:<since>..<until> just\r
+doesn't support seconds since epoch yet. And I think I'll make the\r
+syntax "@<timestamp>" to let you have "<really-big-number> seconds"\r
+without surprises.\r
+\r
+> +EOF\r
+> +\r
+> +${TEST_DIRECTORY}/parse-time --now="Tue Jan 11 11:11:00 +0000 2011" < INPUT > OUTPUT\r
+> +test_expect_equal_file INPUT OUTPUT\r
+> \r
+> test_done\r
+> diff --git a/test/parse-time.c b/test/parse-time.c\r
+> index b4de76b..0415f49 100644\r
+> --- a/test/parse-time.c\r
+> +++ b/test/parse-time.c\r
+> @@ -18,59 +18,47 @@\r
+> * Author: Jani Nikula <jani@nikula.org>\r
+> */\r
+> \r
+> +\r
+> +#define _XOPEN_SOURCE 500 /* for strptime() and snprintf() */\r
+> #include <getopt.h>\r
+> #include <stdio.h>\r
+> #include <stdlib.h>\r
+> #include <string.h>\r
+> +#include <time.h>\r
+> \r
+> #include "parse-time-string.h"\r
+> \r
+> -/*\r
+> - * concat argv[start]...argv[end - 1], separating them by a single\r
+> - * space, to a malloced string\r
+> - */\r
+> -static char *\r
+> -concat_args (int start, int end, char *argv[])\r
+> -{\r
+> - int i;\r
+> - size_t len = 1;\r
+> - char *p;\r
+> -\r
+> - for (i = start; i < end; i++)\r
+> - len += strlen (argv[i]) + 1;\r
+> -\r
+> - p = malloc (len);\r
+> - if (!p)\r
+> - return NULL;\r
+> -\r
+> - *p = 0;\r
+> -\r
+> - for (i = start; i < end; i++) {\r
+> - if (i != start)\r
+> - strcat (p, " ");\r
+> - strcat (p, argv[i]);\r
+> - }\r
+> -\r
+> - return p;\r
+> -}\r
+> -\r
+> #define DEFAULT_FORMAT "%a %b %d %T %z %Y"\r
+> \r
+> static void\r
+> usage (const char *name)\r
+> {\r
+> - printf ("Usage: %s [options ...] <date/time>\n\n", name);\r
+> + printf ("Usage: %s [options ...]\n\n", name);\r
+> printf (\r
+> - "Parse <date/time> and display it in given format.\n\n"\r
+> - " -f, --format=FMT output format, FMT according to strftime(3)\n"\r
+> - " (default: \"%s\")\n"\r
+> - " -n, --now=N use N seconds since epoch as now (default: now)\n"\r
+> - " -u, --up round result up (default: no rounding)\n"\r
+> - " -d, --down round result down (default: no rounding)\n"\r
+> - " -h, --help print this help\n",\r
+> + "Parse date/time read from stdin and display it in given format.\n\n"\r
+> + " -f, --format=FMT output format for dates and input format for --now,\n"\r
+> + " FMT according to strftime(3) (default: \"%s\")\n"\r
+> + " -n, --now=N reference date in FMT (default: now)\n"\r
+> + " -h, --help print this help\n"\r
+> + "\n"\r
+> + "stdin should contain one date/time per line in the following format:\n"\r
+> + " <date/time> [ <arrow> [ comment ] ]\n"\r
+> + "where <arrow> determines the operation performed on the <date/time>.\n"\r
+> + "It can be one of '->', '^->', 'v->' meaning convert, convert and round\n"\r
+> + "up, convert and round down, respectively.\n",\r
+> DEFAULT_FORMAT);\r
+> }\r
+> \r
+> +static const char *\r
+> +get_round_str (int round)\r
+> +{\r
+> + switch (round) {\r
+> + case PARSE_TIME_ROUND_UP: return "^";\r
+> + case PARSE_TIME_ROUND_DOWN: return "v";\r
+> + default: return "";\r
+> + }\r
+> +}\r
+> +\r
+> int\r
+> main (int argc, char *argv[])\r
+> {\r
+> @@ -79,14 +67,10 @@ main (int argc, char *argv[])\r
+> time_t result;\r
+> time_t now;\r
+> time_t *nowp = NULL;\r
+> - char *argstr;\r
+> int round = PARSE_TIME_NO_ROUND;\r
+> - char buf[1024];\r
+> const char *format = DEFAULT_FORMAT;\r
+> struct option options[] = {\r
+> { "help", no_argument, NULL, 'h' },\r
+> - { "up", no_argument, NULL, 'u' },\r
+> - { "down", no_argument, NULL, 'd' },\r
+> { "format", required_argument, NULL, 'f' },\r
+> { "now", required_argument, NULL, 'n' },\r
+> { NULL, 0, NULL, 0 },\r
+> @@ -111,8 +95,13 @@ main (int argc, char *argv[])\r
+> round = PARSE_TIME_ROUND_DOWN;\r
+> break;\r
+> case 'n':\r
+> - /* specify now in seconds since epoch */\r
+> - now = (time_t) strtol (optarg, NULL, 10);\r
+> + memset (&tm, 0, sizeof (tm));\r
+> + char *parsed = strptime (optarg, format, &tm);\r
+\r
+One of the problems with strptime is that it doesn't support time zones,\r
+which is why I chose not to use it here. (You can specify %z in the\r
+format to ignore it, but it looks like it's ignored no matter\r
+what. *shrug*) Combined with mktime below, you introduce possible TZ and\r
+DST variations in the tests, which can be problematic. So perhaps we\r
+should keep the reference time as a timestamp here.\r
+\r
+I didn't look at this test tool patch very closely yet, but in general I\r
+like the very much increased clarity in the tests. I'll look into this\r
+more when I have a moment.\r
+\r
+Thanks for the tests, comments, and corner cases. They've been helpful.\r
+\r
+\r
+BR,\r
+Jani.\r
+\r
+\r
+> + if (!parsed) {\r
+> + fprintf (stderr, "Cannot parse reference date: %s\n", optarg);\r
+> + return 1;\r
+> + }\r
+> + now = mktime (&tm);\r
+> if (now >= (time_t) 0)\r
+> nowp = &now;\r
+> break;\r
+> @@ -124,22 +113,47 @@ main (int argc, char *argv[])\r
+> }\r
+> }\r
+> \r
+> - argstr = concat_args (optind, argc, argv);\r
+> - if (!argstr)\r
+> - return 1;\r
+> -\r
+> - r = parse_time_string (argstr, &result, nowp, round);\r
+> -\r
+> - free (argstr);\r
+> -\r
+> - if (r)\r
+> - return 1;\r
+> -\r
+> - if (!localtime_r (&result, &tm))\r
+> - return 1;\r
+> -\r
+> - strftime (buf, sizeof (buf), format, &tm);\r
+> - printf ("%s\n", buf);\r
+> + char input[BUFSIZ];\r
+> + while (fgets (input, BUFSIZ, stdin) && input[0]) {\r
+> + if (input[0] == '\n') {\r
+> + printf ("\n");\r
+> + continue;\r
+> + }\r
+> + char *arrow;\r
+> + char *comment = strrchr (input, '#');\r
+> + arrow = strstr (input, "->");\r
+> + round = PARSE_TIME_NO_ROUND;\r
+> + if (arrow > input) {\r
+> + switch (arrow[-1]) {\r
+> + case '^': round = PARSE_TIME_ROUND_UP; arrow--; break;\r
+> + case 'v': round = PARSE_TIME_ROUND_DOWN; arrow--; break;\r
+> + default: break;\r
+> + }\r
+> + }\r
+> + if (arrow)\r
+> + *arrow = 0;\r
+> + else\r
+> + arrow = input + strlen (input); /* XXX: comment is not handled */\r
+> + while (arrow > input && arrow[-1] == '\n')\r
+> + arrow--;\r
+> + *arrow-- = 0;\r
+> +\r
+> + r = parse_time_string (input, &result, nowp, round);\r
+> + char resstr[BUFSIZ];\r
+> + if (r)\r
+> + snprintf (resstr, sizeof(resstr), "parse_time_string() error: %d", r);\r
+> + else if (!localtime_r (&result, &tm))\r
+> + snprintf (resstr, sizeof(resstr), "localtime(result) error");\r
+> + else\r
+> + strftime (resstr, sizeof (resstr), format, &tm);\r
+> +\r
+> + char buf[BUFSIZ];\r
+> + snprintf (buf, sizeof(buf), "%s%s-> %s", input, get_round_str (round), resstr);\r
+> + if (!comment)\r
+> + printf ("%s\n", buf);\r
+> + else\r
+> + printf ("%-*s%s", (int)(comment - input), buf, comment);\r
+> + }\r
+> \r
+> return 0;\r
+> }\r
projects / notmuch-archives.git / commitdiff