[PATCH] devel: add release-checks.sh
Currently Makefile.local contains some machine executable release checking functionality. This is unnecessarily complex way to do it: Multiline script functionality is hard to embed -- from Makefile point of view there is just one line split using backslashes and every line ends with ';'. It is hard to maintain such "scripts" when it gets longer. The script does not fail as robust as separate script; set -eu could be added to get same level of robustness -- but the provided Bourne Again Shell (bash) script exceeds this with 'set -o pipefail', making script to fail when any of the commands in pipeline fails (i.e. not just the last one). Checking for release is done very seldom compared to all other use; The whole Makefile.local gets simpler and easier to grasp when most release checking targets are removed. When release checking is done, the steps are executed sequentially; nothing is allowed to be skipped due to some satisfied dependency. --- I did not send patch to remove corresponding lines from Makefile.local just yet. This allows David to run this script in parallel to the code currently in Makefile.local -- and make sure 0.14 gets best release engineering treatment so far. Then Makefile.local can be made to execute this script in place of code it supersedes. devel/release-checks.sh | 210 +++ 1 files changed, 210 insertions(+), 0 deletions(-) create mode 100755 devel/release-checks.sh diff --git a/devel/release-checks.sh b/devel/release-checks.sh new file mode 100755 index 000..29e27df --- /dev/null +++ b/devel/release-checks.sh @@ -0,0 +1,210 @@ +#!/usr/bin/env bash + +set -eu +#set -x # or enter bash -x ... on command line. + +if [ x"${BASH_VERSION-}" = x ] +then echo + echo "Please execute this script using 'bash' interpreter." + echo + exit 1 +fi + +set -o pipefail # bash feature + +# Avoid locale-specific differences in output of executed commands. +LANG=C LC_ALL=C; export LANG LC_ALL + +readonly DEFAULT_IFS="$IFS" + +readonly PV_FILE='bindings/python/notmuch/version.py' + +# using array here turned out to be unnecessarily complicated +emsgs='' +append_emsg () +{ + emsgs="${emsgs:+$emsgs\n} $1" +} + +for f in ./version debian/changelog NEWS "$PV_FILE" +do + test -f $f || { append_emsg "File '$f' is missing"; continue; } + test -r $f || { append_emsg "File '$f' is unreadable"; continue; } + test -s $f || append_emsg "File '$f' is empty" +done + +if [ -n "$emsgs" ] +then + echo 'Release files problems; fix these and try again:' + echo -e "$emsgs" + exit 1 +fi + +if read VERSION +then + if read rest + thenecho "'version' file contains more than one line" + exit 1 + fi +else + echo "Reading './version' file failed (suprisingly!)" + exit 1 +fi < ./version + +readonly VERSION + +verfail () +{ + echo No. + echo "$@" + echo "Please follow the instructions in RELEASING to choose a version" + exit 1 +} + +echo -n "Checking that '$VERSION' is good with digits and periods... " +if [ -z "${VERSION//[0123456789.]/}" ] # bash feature +then + case $VERSION in + .*) verfail "'$VERSION' begins with a period" ;; + *.) verfail "'$VERSION' ends with a period" ;; + *..*) verfail "'$VERSION' contains two consecutive periods";; + *.*)echo Yes. ;; + *) verfail "'$VERSION' is a single number" ;; + esac +else + verfail "'$VERSION' contains other characters than digits and periods" +fi + + +# In the rest of this file, tests collect list of errors to be fixed + +echo -n "Checking that this is Debian package for notmuch... " +read deb_notmuch deb_version rest < debian/changelog +if [ "$deb_notmuch" = 'notmuch' ] +then + echo Yes. +else + echo No. + append_emsg "Package name '$deb_notmuch' is not 'notmuch' in debian/changelog" +fi + +echo -n "Checking that Debian package version is $VERSION-1... " + +if [ "$deb_version" = "($VERSION-1)" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$deb_version' is not '($VERSION-1)' in debian/changelog" +fi + +echo -n "Checking that python bindings version is $VERSION... " +py_version=`python -c "execfile('$PV_FILE'); print __VERSION__"` +if [ "$py_version" = "$VERSION" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$py_version' is not '$VERSION' in $PV_FILE" +fi + +echo -n "Checking that this is Notmuch NEWS... " +read news_notmuch news_version news_date < NEWS +if [ "$news_notmuch" = "Notmuch" ] +then + echo Yes. +else + echo No. + append_emsg "First word '$news_notmuch' is not 'Notmuch' in NEWS file" +fi + +echo -n "Checking that NEWS version is $VERSION... " +if [ "$news_version" = "$VERSION" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$news_version' in NEW
[PATCH 1/4] show: indicate length of omitted body content (json)
On Wed, 08 Aug 2012 00:01:06 +0100, Mark Walters wrote: > > I have two minor queries: do you think omitted text/html parts should > also have the length given? Or even should it always be sent? But I am > happy with it as is. It could be added. I wouldn't have much use for it myself. text/html parts should usually be small enough that it doesn't matter, and I never retrieve them when there is a text/plain alternative. Peter
[PATCH v2 3/3] test: conform to content length, encoding fields
Update tests to expect content-length and content-transfer-encoding fields in show --format=json output, for leaf parts with omitted body content. --- test/crypto| 30 +- test/json |2 +- test/multipart |9 + 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/test/crypto b/test/crypto index 5dd14c4..aa96ec2 100755 --- a/test/crypto +++ b/test/crypto @@ -61,7 +61,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -95,7 +96,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -127,7 +129,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -196,7 +199,8 @@ expected='[[[{"id": "X", "sigstatus": [], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, "content-type": "multipart/mixed", "content": [{"id": 4, @@ -204,6 +208,8 @@ expected='[[[{"id": "X", "content": "This is a test encrypted message.\n"}, {"id": 5, "content-type": "application/octet-stream", + "content-length": 28, + "content-transfer-encoding": "base64", "filename": "TESTATTACHMENT"}]}]}]}, [' test_expect_equal_json \ @@ -231,9 +237,11 @@ test_expect_equal_file OUTPUT TESTATTACHMENT test_begin_subtest "decryption failure with missing key" mv "${GNUPGHOME}"{,.bak} +# The length of the encrypted attachment varies so must be normalized. output=$(notmuch show --format=json --decrypt subject:"test encrypted message 001" \ | notmuch_json_show_sanitize \ -| sed -e 's|"created": [1234567890]*|"created": 946728000|') +| sed -e 's|"created": [1234567890]*|"created": 946728000|' \ +| sed -e 's|"content-length": 6[1234567890]*|"content-length": 652|') expected='[[[{"id": "X", "match": true, "excluded": false, @@ -249,9 +257,11 @@ expected='[[[{"id": "X", "encstatus": [{"status": "bad"}], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, - "content-type": "application/octet-stream"}]}]}, + "content-type": "application/octet-stream", + "content-length": 652}]}]}, [' test_expect_equal_json \ "$output" \ @@ -287,7 +297,8 @@ expected='[[[{"id": "X", "userid": " Notmuch Test Suite (INSECURE!)"}], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, "content-type": "text/plain", "content": "This is another test encrypted message.\n"}]}]}, @@ -342,7 +353,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ diff --git a/test/json b/test/json index ac8fa8e..8ce2e8a 100755 --- a/test/json +++ b/test/json @@ -45,7 +45,7 @@ emacs_deliver_message \ (insert \"Message-ID: <$id>\n\")" output=$(notmuch show --format=json "id:$id") filename=$(notmuch search --output=files "id:$id") -test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite \", \"To\": \"test_suite at notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, [" +test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {
[PATCH v2 2/3] show: indicate length, encoding of omitted body content
If a leaf part's body content is omitted, return the encoded length and transfer encoding in --format=json output. This information may be used by the consumer, e.g. to decide whether to download a large attachment over a slow link. Returning the _encoded_ content length is more efficient than returning the _decoded_ content length. Returning the transfer encoding allows the consumer to estimate the decoded content length. --- devel/schemata |9 - notmuch-show.c | 14 ++ 2 files changed, 22 insertions(+), 1 deletions(-) diff --git a/devel/schemata b/devel/schemata index 9cb25f5..094b9a5 100644 --- a/devel/schemata +++ b/devel/schemata @@ -69,7 +69,14 @@ part = { # A leaf part's body content is optional, but may be included if # it can be correctly encoded as a string. Consumers should use # this in preference to fetching the part content separately. -content?: string +content?: string, +# If a leaf part's body content is not included, the length of +# the encoded content (in bytes) may be given instead. +content-length?: int, +# If a leaf part's body content is not included, its transfer encoding +# may be given. Using this and the encoded content length, it is +# possible for the consumer to estimate the decoded content length. +content-transfer-encoding?: string } # The headers of a message or part (format_headers_json with reply = FALSE) diff --git a/notmuch-show.c b/notmuch-show.c index 3556293..83535c7 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -664,6 +664,20 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->map_key (sp, "content"); sp->string_len (sp, (char *) part_content->data, part_content->len); g_object_unref (stream_memory); + } else { + GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part)); + GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper); + ssize_t length = g_mime_stream_length (stream); + const char *cte = g_mime_object_get_header (meta, "content-transfer-encoding"); + + if (length >= 0) { + sp->map_key (sp, "content-length"); + sp->integer (sp, length); + } + if (cte) { + sp->map_key (sp, "content-transfer-encoding"); + sp->string (sp, cte); + } } } else if (GMIME_IS_MULTIPART (node->part)) { sp->map_key (sp, "content"); -- 1.7.4.4
[PATCH v2 1/3] test: normalize only message filenames in show json
notmuch_json_show_sanitize replaced "filename" field values even in part structures, where the value is predictable. Make it only normalize the filename value if it is an absolute path (begins with slash), which is true of the Maildir filenames that were intended to be normalized away. --- test/multipart |2 +- test/test-lib.sh |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/multipart b/test/multipart index 0527f84..344ed81 100755 --- a/test/multipart +++ b/test/multipart @@ -630,7 +630,7 @@ cat
[PATCH v2 0/3] indicate length of omitted body content
The first commit is tangentally related. An expected test case output in test/crypto previously had a filename left unnormalised by notmuch_json_show_sanitize because it was not followed by comma. The next commit causes the comma to be present, breaking the expected output. In the 2nd commit, a content-transfer-encoding field was added and comments clarified. In the 3rd commit, the content-length of an encrypted attachment had to be normalised because it varies between runs. Peter Wang (3): test: normalize only message filenames in show json show: indicate length, encoding of omitted body content test: conform to content length, encoding fields devel/schemata |9 - notmuch-show.c | 14 ++ test/crypto | 30 +- test/json|2 +- test/multipart | 11 ++- test/test-lib.sh |2 +- 6 files changed, 51 insertions(+), 17 deletions(-) -- 1.7.4.4
[PATCH] sprinters: bugfix when NULL passed for a string.
The string function in a sprinter may be called with a NULL string pointer (eg if a header is absent). This causes a segfault. We fix this by checking for a null pointer in the string functions and update the sprinter documentation. At the moment some output when format=text is done directly rather than via an sprinter: in that case a null pointer is passed to printf or similar and a "(null)" appears in the output. That behaviour is not changed in this patch. --- This is the same as id:"874noe1o0r.fsf at qmul.ac.uk" except for being based on top of the test in the parent message, and marking the broken test fixed. Best wishes Mark sprinter-json.c |2 ++ sprinter-text.c |2 ++ sprinter.h |4 +++- test/missing-headers |2 -- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sprinter-json.c b/sprinter-json.c index c9b6835..0a07790 100644 --- a/sprinter-json.c +++ b/sprinter-json.c @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, size_t len) static void json_string (struct sprinter *sp, const char *val) { +if (val == NULL) + val = ""; json_string_len (sp, val, strlen (val)); } diff --git a/sprinter-text.c b/sprinter-text.c index dfa54b5..10343be 100644 --- a/sprinter-text.c +++ b/sprinter-text.c @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, size_t len) static void text_string (struct sprinter *sp, const char *val) { +if (val == NULL) + val = ""; text_string_len (sp, val, strlen (val)); } diff --git a/sprinter.h b/sprinter.h index 5f43175..912a526 100644 --- a/sprinter.h +++ b/sprinter.h @@ -27,7 +27,9 @@ typedef struct sprinter { * a list or map, followed or preceded by separators). For string * and string_len, the char * must be UTF-8 encoded. string_len * allows non-terminated strings and strings with embedded NULs - * (though the handling of the latter is format-dependent). + * (though the handling of the latter is format-dependent). For + * string (but not string_len) the string pointer passed may be + * NULL. */ void (*string) (struct sprinter *, const char *); void (*string_len) (struct sprinter *, const char *, size_t); diff --git a/test/missing-headers b/test/missing-headers index e79f922..f14b878 100755 --- a/test/missing-headers +++ b/test/missing-headers @@ -29,7 +29,6 @@ thread:XXX 2001-01-05 [1/1] (null); (inbox unread) thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" test_begin_subtest "Search: json" -test_subtest_known_broken output=$(notmuch search --format=json '*' | notmuch_search_sanitize) test_expect_equal_json "$output" ' [ @@ -93,7 +92,6 @@ Body message}" test_begin_subtest "Show: json" -test_subtest_known_broken output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) test_expect_equal_json "$output" ' [ -- 1.7.9.1
[PATCH v2] test: Add test for messages with missing headers
On Wed, Aug 08 2012, Austin Clements wrote: > Quoth Tomi Ollila on Aug 08 at 11:08 am: >> On Wed, Aug 08 2012, Austin Clements wrote: >> >> > Currently the JSON tests for search and show are broken because >> > notmuch attempts to dereference a NULL pointer. >> > --- >> > This version fixes the "Show: text" test so that it sanitize its >> > output and doesn't hard-code my test paths. >> >> +1 >> >> Tomi >> >> w/ python json(.tool) the original order cannot be preserved as the parser >> stores content into dictionary -- without sorting those came in some >> internal python order... the following code could be used to use less > > To be fair, it can output them in original order, but doing so > requires Python 2.7: > > python -c 'import sys,json,collections; json.dump(json.load(sys.stdin, > object_pairs_hook=collections.OrderedDict), sys.stdout, indent=4)' ack. > >> indentation, though: >> >> python -c 'import sys,json; j = json.load(sys.stdin); >> json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file >> >> The other "problem" with json.tool is that it converts non-ascii chars >> to \u values :/. > > This one doesn't require 2.7. > > python -c 'import sys,json,codecs; json.dump(json.load(sys.stdin), > codecs.getwriter("utf8")(sys.stdout), indent=4, ensure_ascii=False)' nice. > > Though I think that, for test canonicalization, \u is probably > less error/locale prone. true. > >> What we could do is to dig a simple c json formatter -- someday in the >> future, maybe -- but for *now* this is the best we can have :D > > Not another JSON parser/printer! Why not, one from scratch, without looking any source there is already >;) Well, you're right; the json.tool version python 2.6 provides is good -- -- ordering and those \u:s doesn't really matter. Probably thinking (or planning!) anything else at this time is waste of time. At least these 2 questions remain: Is the test requirement of python 2.6 suitable/acceptable ? How do we pretty-print S-expression syntax ;) ? Tomi > >> > >> > test/missing-headers | 162 >> > ++ >> > test/notmuch-test|1 + >> > 2 files changed, 163 insertions(+) >> > create mode 100755 test/missing-headers >> > >> > diff --git a/test/missing-headers b/test/missing-headers >> > new file mode 100755 >> > index 000..e79f922 >> > --- /dev/null >> > +++ b/test/missing-headers >> > @@ -0,0 +1,162 @@ >> > +#!/usr/bin/env bash >> > +test_description='messages with missing headers' >> > +. ./test-lib.sh >> > + >> > +# Notmuch requires at least one of from, subject, or to or it will >> > +# ignore the file. Generate two messages so that together they cover >> > +# all possible missing headers. We also give one of the messages a >> > +# date to ensure stable result ordering. >> > + >> > +cat < "${MAIL_DIR}/msg-2" >> > +To: Notmuch Test Suite >> > +Date: Fri, 05 Jan 2001 15:43:57 + >> > + >> > +Body >> > +EOF >> > + >> > +cat < "${MAIL_DIR}/msg-1" >> > +From: Notmuch Test Suite >> > + >> > +Body >> > +EOF >> > + >> > +NOTMUCH_NEW >> > + >> > +test_begin_subtest "Search: text" >> > +output=$(notmuch search '*' | notmuch_search_sanitize) >> > +test_expect_equal "$output" "\ >> > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) >> > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" >> > + >> > +test_begin_subtest "Search: json" >> > +test_subtest_known_broken >> > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) >> > +test_expect_equal_json "$output" ' >> > +[ >> > +{ >> > +"authors": "", >> > +"date_relative": "2001-01-05", >> > +"matched": 1, >> > +"subject": "", >> > +"tags": [ >> > +"inbox", >> > +"unread" >> > +], >> > +"thread": "XXX", >> > +"timestamp": 978709437, >> > +"total": 1 >> > +}, >> > +{ >> > +"authors": "Notmuch Test Suite", >> > +"date_relative": "1970-01-01", >> > +"matched": 1, >> > +"subject": "", >> > +"tags": [ >> > +"inbox", >> > +"unread" >> > +], >> > +"thread": "XXX", >> > +"timestamp": 0, >> > +"total": 1 >> > +} >> > +]' >> > + >> > +test_begin_subtest "Show: text" >> > +output=$(notmuch show '*' | notmuch_show_sanitize) >> > +test_expect_equal "$output" "\ >> > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 >> > depth:0 match:1 excluded:0 filename:/XXX/mail/msg-2 >> > +header{ >> > + (2001-01-05) (inbox unread) >> > +Subject: (null) >> > +From: (null) >> > +To: Notmuch Test Suite >> > +Date: Fri, 05 Jan 2001 15:43:57 + >> > +header} >> > +body{ >> > +part{ ID: 1, Content-type: text/plain >> > +Body >> > +part} >> > +body} >> > +message} >> > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 >> > depth:0 match:1 excluded:0 filename:/XXX/mail/msg-1 >> > +header
[PATCH] emacs: Fix "not defined at runtime" warning
Previously, the Emacs byte compiler produced the warning the function `remove-if-not' might not be defined at runtime. because we only required cl at compile-time (not runtime). This fixes this warning by requiring cl at runtime, ensuring that the definition of remove-if-not is available. --- emacs/notmuch-lib.el |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 30db58f..900235b 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -24,7 +24,7 @@ (require 'mm-view) (require 'mm-decode) (require 'json) -(eval-when-compile (require 'cl)) +(require 'cl) (defvar notmuch-command "notmuch" "Command to run the notmuch binary.") -- 1.7.10
[PATCH] sprinters: bugfix when NULL passed for a string.
LGTM! Quoth Mark Walters on Aug 08 at 10:23 pm: > > The string function in a sprinter may be called with a NULL string > pointer (eg if a header is absent). This causes a segfault. We fix > this by checking for a null pointer in the string functions and update > the sprinter documentation. > > At the moment some output when format=text is done directly rather than > via an sprinter: in that case a null pointer is passed to printf or > similar and a "(null)" appears in the output. That behaviour is not > changed in this patch. > --- > > This is the same as id:"874noe1o0r.fsf at qmul.ac.uk" except for being > based on top of the test in the parent message, and marking the broken > test fixed. > > Best wishes > > Mark > > sprinter-json.c |2 ++ > sprinter-text.c |2 ++ > sprinter.h |4 +++- > test/missing-headers |2 -- > 4 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/sprinter-json.c b/sprinter-json.c > index c9b6835..0a07790 100644 > --- a/sprinter-json.c > +++ b/sprinter-json.c > @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, > size_t len) > static void > json_string (struct sprinter *sp, const char *val) > { > +if (val == NULL) > + val = ""; > json_string_len (sp, val, strlen (val)); > } > > diff --git a/sprinter-text.c b/sprinter-text.c > index dfa54b5..10343be 100644 > --- a/sprinter-text.c > +++ b/sprinter-text.c > @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, > size_t len) > static void > text_string (struct sprinter *sp, const char *val) > { > +if (val == NULL) > + val = ""; > text_string_len (sp, val, strlen (val)); > } > > diff --git a/sprinter.h b/sprinter.h > index 5f43175..912a526 100644 > --- a/sprinter.h > +++ b/sprinter.h > @@ -27,7 +27,9 @@ typedef struct sprinter { > * a list or map, followed or preceded by separators). For string > * and string_len, the char * must be UTF-8 encoded. string_len > * allows non-terminated strings and strings with embedded NULs > - * (though the handling of the latter is format-dependent). > + * (though the handling of the latter is format-dependent). For > + * string (but not string_len) the string pointer passed may be > + * NULL. > */ > void (*string) (struct sprinter *, const char *); > void (*string_len) (struct sprinter *, const char *, size_t); > diff --git a/test/missing-headers b/test/missing-headers > index e79f922..f14b878 100755 > --- a/test/missing-headers > +++ b/test/missing-headers > @@ -29,7 +29,6 @@ thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > > test_begin_subtest "Search: json" > -test_subtest_known_broken > output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > test_expect_equal_json "$output" ' > [ > @@ -93,7 +92,6 @@ Body > message}" > > test_begin_subtest "Show: json" > -test_subtest_known_broken > output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > test_expect_equal_json "$output" ' > [
[PATCH v2] test: Add test for messages with missing headers
Quoth Tomi Ollila on Aug 08 at 9:47 pm: > On Wed, Aug 08 2012, Austin Clements wrote: > > Not another JSON parser/printer! > > Why not, one from scratch, without looking any source there is already >;) > > Well, you're right; the json.tool version python 2.6 provides is good -- > -- ordering and those \u:s doesn't really matter. > Probably thinking (or planning!) anything else at this time is waste of time. > > At least these 2 questions remain: > > Is the test requirement of python 2.6 suitable/acceptable ? Too late; it's already in master. Besides, even *Debian stable* has Python 2.6, which qualifies it as ubiquitous in my book. > How do we pretty-print S-expression syntax ;) ? Emacs of course! echo '(1 2 (3 4) (5 6))' | \ emacs -Q --batch --eval '(pp (read-from-minibuffer "" nil nil t))' This may or may not be a good idea. > Tomi
[PATCH] emacs: Fix "not defined at runtime" warning
Previously, the Emacs byte compiler produced the warning the function `remove-if-not' might not be defined at runtime. because we only required cl at compile-time (not runtime). This fixes this warning by requiring cl at runtime, ensuring that the definition of remove-if-not is available. --- emacs/notmuch-lib.el |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el index 30db58f..900235b 100644 --- a/emacs/notmuch-lib.el +++ b/emacs/notmuch-lib.el @@ -24,7 +24,7 @@ (require 'mm-view) (require 'mm-decode) (require 'json) -(eval-when-compile (require 'cl)) +(require 'cl) (defvar notmuch-command "notmuch" "Command to run the notmuch binary.") -- 1.7.10 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] sprinters: bugfix when NULL passed for a string.
LGTM! Quoth Mark Walters on Aug 08 at 10:23 pm: > > The string function in a sprinter may be called with a NULL string > pointer (eg if a header is absent). This causes a segfault. We fix > this by checking for a null pointer in the string functions and update > the sprinter documentation. > > At the moment some output when format=text is done directly rather than > via an sprinter: in that case a null pointer is passed to printf or > similar and a "(null)" appears in the output. That behaviour is not > changed in this patch. > --- > > This is the same as id:"874noe1o0r@qmul.ac.uk" except for being > based on top of the test in the parent message, and marking the broken > test fixed. > > Best wishes > > Mark > > sprinter-json.c |2 ++ > sprinter-text.c |2 ++ > sprinter.h |4 +++- > test/missing-headers |2 -- > 4 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/sprinter-json.c b/sprinter-json.c > index c9b6835..0a07790 100644 > --- a/sprinter-json.c > +++ b/sprinter-json.c > @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, > size_t len) > static void > json_string (struct sprinter *sp, const char *val) > { > +if (val == NULL) > + val = ""; > json_string_len (sp, val, strlen (val)); > } > > diff --git a/sprinter-text.c b/sprinter-text.c > index dfa54b5..10343be 100644 > --- a/sprinter-text.c > +++ b/sprinter-text.c > @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, > size_t len) > static void > text_string (struct sprinter *sp, const char *val) > { > +if (val == NULL) > + val = ""; > text_string_len (sp, val, strlen (val)); > } > > diff --git a/sprinter.h b/sprinter.h > index 5f43175..912a526 100644 > --- a/sprinter.h > +++ b/sprinter.h > @@ -27,7 +27,9 @@ typedef struct sprinter { > * a list or map, followed or preceded by separators). For string > * and string_len, the char * must be UTF-8 encoded. string_len > * allows non-terminated strings and strings with embedded NULs > - * (though the handling of the latter is format-dependent). > + * (though the handling of the latter is format-dependent). For > + * string (but not string_len) the string pointer passed may be > + * NULL. > */ > void (*string) (struct sprinter *, const char *); > void (*string_len) (struct sprinter *, const char *, size_t); > diff --git a/test/missing-headers b/test/missing-headers > index e79f922..f14b878 100755 > --- a/test/missing-headers > +++ b/test/missing-headers > @@ -29,7 +29,6 @@ thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > > test_begin_subtest "Search: json" > -test_subtest_known_broken > output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > test_expect_equal_json "$output" ' > [ > @@ -93,7 +92,6 @@ Body > message}" > > test_begin_subtest "Show: json" > -test_subtest_known_broken > output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > test_expect_equal_json "$output" ' > [ ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] sprinters: bugfix when NULL passed for a string.
The string function in a sprinter may be called with a NULL string pointer (eg if a header is absent). This causes a segfault. We fix this by checking for a null pointer in the string functions and update the sprinter documentation. At the moment some output when format=text is done directly rather than via an sprinter: in that case a null pointer is passed to printf or similar and a "(null)" appears in the output. That behaviour is not changed in this patch. --- This is the same as id:"874noe1o0r@qmul.ac.uk" except for being based on top of the test in the parent message, and marking the broken test fixed. Best wishes Mark sprinter-json.c |2 ++ sprinter-text.c |2 ++ sprinter.h |4 +++- test/missing-headers |2 -- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sprinter-json.c b/sprinter-json.c index c9b6835..0a07790 100644 --- a/sprinter-json.c +++ b/sprinter-json.c @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, size_t len) static void json_string (struct sprinter *sp, const char *val) { +if (val == NULL) + val = ""; json_string_len (sp, val, strlen (val)); } diff --git a/sprinter-text.c b/sprinter-text.c index dfa54b5..10343be 100644 --- a/sprinter-text.c +++ b/sprinter-text.c @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, size_t len) static void text_string (struct sprinter *sp, const char *val) { +if (val == NULL) + val = ""; text_string_len (sp, val, strlen (val)); } diff --git a/sprinter.h b/sprinter.h index 5f43175..912a526 100644 --- a/sprinter.h +++ b/sprinter.h @@ -27,7 +27,9 @@ typedef struct sprinter { * a list or map, followed or preceded by separators). For string * and string_len, the char * must be UTF-8 encoded. string_len * allows non-terminated strings and strings with embedded NULs - * (though the handling of the latter is format-dependent). + * (though the handling of the latter is format-dependent). For + * string (but not string_len) the string pointer passed may be + * NULL. */ void (*string) (struct sprinter *, const char *); void (*string_len) (struct sprinter *, const char *, size_t); diff --git a/test/missing-headers b/test/missing-headers index e79f922..f14b878 100755 --- a/test/missing-headers +++ b/test/missing-headers @@ -29,7 +29,6 @@ thread:XXX 2001-01-05 [1/1] (null); (inbox unread) thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" test_begin_subtest "Search: json" -test_subtest_known_broken output=$(notmuch search --format=json '*' | notmuch_search_sanitize) test_expect_equal_json "$output" ' [ @@ -93,7 +92,6 @@ Body message}" test_begin_subtest "Show: json" -test_subtest_known_broken output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) test_expect_equal_json "$output" ' [ -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] devel: add release-checks.sh
Currently Makefile.local contains some machine executable release checking functionality. This is unnecessarily complex way to do it: Multiline script functionality is hard to embed -- from Makefile point of view there is just one line split using backslashes and every line ends with ';'. It is hard to maintain such "scripts" when it gets longer. The script does not fail as robust as separate script; set -eu could be added to get same level of robustness -- but the provided Bourne Again Shell (bash) script exceeds this with 'set -o pipefail', making script to fail when any of the commands in pipeline fails (i.e. not just the last one). Checking for release is done very seldom compared to all other use; The whole Makefile.local gets simpler and easier to grasp when most release checking targets are removed. When release checking is done, the steps are executed sequentially; nothing is allowed to be skipped due to some satisfied dependency. --- I did not send patch to remove corresponding lines from Makefile.local just yet. This allows David to run this script in parallel to the code currently in Makefile.local -- and make sure 0.14 gets best release engineering treatment so far. Then Makefile.local can be made to execute this script in place of code it supersedes. devel/release-checks.sh | 210 +++ 1 files changed, 210 insertions(+), 0 deletions(-) create mode 100755 devel/release-checks.sh diff --git a/devel/release-checks.sh b/devel/release-checks.sh new file mode 100755 index 000..29e27df --- /dev/null +++ b/devel/release-checks.sh @@ -0,0 +1,210 @@ +#!/usr/bin/env bash + +set -eu +#set -x # or enter bash -x ... on command line. + +if [ x"${BASH_VERSION-}" = x ] +then echo + echo "Please execute this script using 'bash' interpreter." + echo + exit 1 +fi + +set -o pipefail # bash feature + +# Avoid locale-specific differences in output of executed commands. +LANG=C LC_ALL=C; export LANG LC_ALL + +readonly DEFAULT_IFS="$IFS" + +readonly PV_FILE='bindings/python/notmuch/version.py' + +# using array here turned out to be unnecessarily complicated +emsgs='' +append_emsg () +{ + emsgs="${emsgs:+$emsgs\n} $1" +} + +for f in ./version debian/changelog NEWS "$PV_FILE" +do + test -f $f || { append_emsg "File '$f' is missing"; continue; } + test -r $f || { append_emsg "File '$f' is unreadable"; continue; } + test -s $f || append_emsg "File '$f' is empty" +done + +if [ -n "$emsgs" ] +then + echo 'Release files problems; fix these and try again:' + echo -e "$emsgs" + exit 1 +fi + +if read VERSION +then + if read rest + thenecho "'version' file contains more than one line" + exit 1 + fi +else + echo "Reading './version' file failed (suprisingly!)" + exit 1 +fi < ./version + +readonly VERSION + +verfail () +{ + echo No. + echo "$@" + echo "Please follow the instructions in RELEASING to choose a version" + exit 1 +} + +echo -n "Checking that '$VERSION' is good with digits and periods... " +if [ -z "${VERSION//[0123456789.]/}" ] # bash feature +then + case $VERSION in + .*) verfail "'$VERSION' begins with a period" ;; + *.) verfail "'$VERSION' ends with a period" ;; + *..*) verfail "'$VERSION' contains two consecutive periods";; + *.*)echo Yes. ;; + *) verfail "'$VERSION' is a single number" ;; + esac +else + verfail "'$VERSION' contains other characters than digits and periods" +fi + + +# In the rest of this file, tests collect list of errors to be fixed + +echo -n "Checking that this is Debian package for notmuch... " +read deb_notmuch deb_version rest < debian/changelog +if [ "$deb_notmuch" = 'notmuch' ] +then + echo Yes. +else + echo No. + append_emsg "Package name '$deb_notmuch' is not 'notmuch' in debian/changelog" +fi + +echo -n "Checking that Debian package version is $VERSION-1... " + +if [ "$deb_version" = "($VERSION-1)" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$deb_version' is not '($VERSION-1)' in debian/changelog" +fi + +echo -n "Checking that python bindings version is $VERSION... " +py_version=`python -c "execfile('$PV_FILE'); print __VERSION__"` +if [ "$py_version" = "$VERSION" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$py_version' is not '$VERSION' in $PV_FILE" +fi + +echo -n "Checking that this is Notmuch NEWS... " +read news_notmuch news_version news_date < NEWS +if [ "$news_notmuch" = "Notmuch" ] +then + echo Yes. +else + echo No. + append_emsg "First word '$news_notmuch' is not 'Notmuch' in NEWS file" +fi + +echo -n "Checking that NEWS version is $VERSION... " +if [ "$news_version" = "$VERSION" ] +then + echo Yes. +else + echo No. + append_emsg "Version '$news_version' in NEW
Re: [PATCH v2] test: Add test for messages with missing headers
Quoth Tomi Ollila on Aug 08 at 9:47 pm: > On Wed, Aug 08 2012, Austin Clements wrote: > > Not another JSON parser/printer! > > Why not, one from scratch, without looking any source there is already >;) > > Well, you're right; the json.tool version python 2.6 provides is good -- > -- ordering and those \u:s doesn't really matter. > Probably thinking (or planning!) anything else at this time is waste of time. > > At least these 2 questions remain: > > Is the test requirement of python 2.6 suitable/acceptable ? Too late; it's already in master. Besides, even *Debian stable* has Python 2.6, which qualifies it as ubiquitous in my book. > How do we pretty-print S-expression syntax ;) ? Emacs of course! echo '(1 2 (3 4) (5 6))' | \ emacs -Q --batch --eval '(pp (read-from-minibuffer "" nil nil t))' This may or may not be a good idea. > Tomi ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] test: Add test for messages with missing headers
On Wed, Aug 08 2012, Austin Clements wrote: > Quoth Tomi Ollila on Aug 08 at 11:08 am: >> On Wed, Aug 08 2012, Austin Clements wrote: >> >> > Currently the JSON tests for search and show are broken because >> > notmuch attempts to dereference a NULL pointer. >> > --- >> > This version fixes the "Show: text" test so that it sanitize its >> > output and doesn't hard-code my test paths. >> >> +1 >> >> Tomi >> >> w/ python json(.tool) the original order cannot be preserved as the parser >> stores content into dictionary -- without sorting those came in some >> internal python order... the following code could be used to use less > > To be fair, it can output them in original order, but doing so > requires Python 2.7: > > python -c 'import sys,json,collections; json.dump(json.load(sys.stdin, > object_pairs_hook=collections.OrderedDict), sys.stdout, indent=4)' ack. > >> indentation, though: >> >> python -c 'import sys,json; j = json.load(sys.stdin); >> json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file >> >> The other "problem" with json.tool is that it converts non-ascii chars >> to \u values :/. > > This one doesn't require 2.7. > > python -c 'import sys,json,codecs; json.dump(json.load(sys.stdin), > codecs.getwriter("utf8")(sys.stdout), indent=4, ensure_ascii=False)' nice. > > Though I think that, for test canonicalization, \u is probably > less error/locale prone. true. > >> What we could do is to dig a simple c json formatter -- someday in the >> future, maybe -- but for *now* this is the best we can have :D > > Not another JSON parser/printer! Why not, one from scratch, without looking any source there is already >;) Well, you're right; the json.tool version python 2.6 provides is good -- -- ordering and those \u:s doesn't really matter. Probably thinking (or planning!) anything else at this time is waste of time. At least these 2 questions remain: Is the test requirement of python 2.6 suitable/acceptable ? How do we pretty-print S-expression syntax ;) ? Tomi > >> > >> > test/missing-headers | 162 >> > ++ >> > test/notmuch-test|1 + >> > 2 files changed, 163 insertions(+) >> > create mode 100755 test/missing-headers >> > >> > diff --git a/test/missing-headers b/test/missing-headers >> > new file mode 100755 >> > index 000..e79f922 >> > --- /dev/null >> > +++ b/test/missing-headers >> > @@ -0,0 +1,162 @@ >> > +#!/usr/bin/env bash >> > +test_description='messages with missing headers' >> > +. ./test-lib.sh >> > + >> > +# Notmuch requires at least one of from, subject, or to or it will >> > +# ignore the file. Generate two messages so that together they cover >> > +# all possible missing headers. We also give one of the messages a >> > +# date to ensure stable result ordering. >> > + >> > +cat < "${MAIL_DIR}/msg-2" >> > +To: Notmuch Test Suite >> > +Date: Fri, 05 Jan 2001 15:43:57 + >> > + >> > +Body >> > +EOF >> > + >> > +cat < "${MAIL_DIR}/msg-1" >> > +From: Notmuch Test Suite >> > + >> > +Body >> > +EOF >> > + >> > +NOTMUCH_NEW >> > + >> > +test_begin_subtest "Search: text" >> > +output=$(notmuch search '*' | notmuch_search_sanitize) >> > +test_expect_equal "$output" "\ >> > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) >> > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" >> > + >> > +test_begin_subtest "Search: json" >> > +test_subtest_known_broken >> > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) >> > +test_expect_equal_json "$output" ' >> > +[ >> > +{ >> > +"authors": "", >> > +"date_relative": "2001-01-05", >> > +"matched": 1, >> > +"subject": "", >> > +"tags": [ >> > +"inbox", >> > +"unread" >> > +], >> > +"thread": "XXX", >> > +"timestamp": 978709437, >> > +"total": 1 >> > +}, >> > +{ >> > +"authors": "Notmuch Test Suite", >> > +"date_relative": "1970-01-01", >> > +"matched": 1, >> > +"subject": "", >> > +"tags": [ >> > +"inbox", >> > +"unread" >> > +], >> > +"thread": "XXX", >> > +"timestamp": 0, >> > +"total": 1 >> > +} >> > +]' >> > + >> > +test_begin_subtest "Show: text" >> > +output=$(notmuch show '*' | notmuch_show_sanitize) >> > +test_expect_equal "$output" "\ >> > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 >> > depth:0 match:1 excluded:0 filename:/XXX/mail/msg-2 >> > +header{ >> > + (2001-01-05) (inbox unread) >> > +Subject: (null) >> > +From: (null) >> > +To: Notmuch Test Suite >> > +Date: Fri, 05 Jan 2001 15:43:57 + >> > +header} >> > +body{ >> > +part{ ID: 1, Content-type: text/plain >> > +Body >> > +part} >> > +body} >> > +message} >> > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 >> > depth:0 match:1 excluded:0 filename:/XXX/mail/msg-1 >> > +header
[PATCH v2] test: Add test for messages with missing headers
Quoth Tomi Ollila on Aug 08 at 11:08 am: > On Wed, Aug 08 2012, Austin Clements wrote: > > > Currently the JSON tests for search and show are broken because > > notmuch attempts to dereference a NULL pointer. > > --- > > This version fixes the "Show: text" test so that it sanitize its > > output and doesn't hard-code my test paths. > > +1 > > Tomi > > w/ python json(.tool) the original order cannot be preserved as the parser > stores content into dictionary -- without sorting those came in some > internal python order... the following code could be used to use less To be fair, it can output them in original order, but doing so requires Python 2.7: python -c 'import sys,json,collections; json.dump(json.load(sys.stdin, object_pairs_hook=collections.OrderedDict), sys.stdout, indent=4)' > indentation, though: > > python -c 'import sys,json; j = json.load(sys.stdin); > json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file > > The other "problem" with json.tool is that it converts non-ascii chars > to \u values :/. This one doesn't require 2.7. python -c 'import sys,json,codecs; json.dump(json.load(sys.stdin), codecs.getwriter("utf8")(sys.stdout), indent=4, ensure_ascii=False)' Though I think that, for test canonicalization, \u is probably less error/locale prone. > What we could do is to dig a simple c json formatter -- someday in the > future, maybe -- but for *now* this is the best we can have :D Not another JSON parser/printer! > > > > test/missing-headers | 162 > > ++ > > test/notmuch-test|1 + > > 2 files changed, 163 insertions(+) > > create mode 100755 test/missing-headers > > > > diff --git a/test/missing-headers b/test/missing-headers > > new file mode 100755 > > index 000..e79f922 > > --- /dev/null > > +++ b/test/missing-headers > > @@ -0,0 +1,162 @@ > > +#!/usr/bin/env bash > > +test_description='messages with missing headers' > > +. ./test-lib.sh > > + > > +# Notmuch requires at least one of from, subject, or to or it will > > +# ignore the file. Generate two messages so that together they cover > > +# all possible missing headers. We also give one of the messages a > > +# date to ensure stable result ordering. > > + > > +cat < "${MAIL_DIR}/msg-2" > > +To: Notmuch Test Suite > > +Date: Fri, 05 Jan 2001 15:43:57 + > > + > > +Body > > +EOF > > + > > +cat < "${MAIL_DIR}/msg-1" > > +From: Notmuch Test Suite > > + > > +Body > > +EOF > > + > > +NOTMUCH_NEW > > + > > +test_begin_subtest "Search: text" > > +output=$(notmuch search '*' | notmuch_search_sanitize) > > +test_expect_equal "$output" "\ > > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > > + > > +test_begin_subtest "Search: json" > > +test_subtest_known_broken > > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > > +test_expect_equal_json "$output" ' > > +[ > > +{ > > +"authors": "", > > +"date_relative": "2001-01-05", > > +"matched": 1, > > +"subject": "", > > +"tags": [ > > +"inbox", > > +"unread" > > +], > > +"thread": "XXX", > > +"timestamp": 978709437, > > +"total": 1 > > +}, > > +{ > > +"authors": "Notmuch Test Suite", > > +"date_relative": "1970-01-01", > > +"matched": 1, > > +"subject": "", > > +"tags": [ > > +"inbox", > > +"unread" > > +], > > +"thread": "XXX", > > +"timestamp": 0, > > +"total": 1 > > +} > > +]' > > + > > +test_begin_subtest "Show: text" > > +output=$(notmuch show '*' | notmuch_show_sanitize) > > +test_expect_equal "$output" "\ > > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > > match:1 excluded:0 filename:/XXX/mail/msg-2 > > +header{ > > + (2001-01-05) (inbox unread) > > +Subject: (null) > > +From: (null) > > +To: Notmuch Test Suite > > +Date: Fri, 05 Jan 2001 15:43:57 + > > +header} > > +body{ > > +part{ ID: 1, Content-type: text/plain > > +Body > > +part} > > +body} > > +message} > > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > > match:1 excluded:0 filename:/XXX/mail/msg-1 > > +header{ > > +Notmuch Test Suite (1970-01-01) (inbox > > unread) > > +Subject: (null) > > +From: Notmuch Test Suite > > +Date: Thu, 01 Jan 1970 00:00:00 + > > +header} > > +body{ > > +part{ ID: 1, Content-type: text/plain > > +Body > > +part} > > +body} > > +message}" > > + > > +test_begin_subtest "Show: json" > > +test_subtest_known_broken > > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > > +test_expect_equal_json "$output" ' > > +[ > > +[ > > +[ > > +{ > > +"body": [ > > +{ > > +"content": "Body\n", > > +"co
[PATCH v2] test: Add test for messages with missing headers
On Wed, Aug 08 2012, Austin Clements wrote: > Currently the JSON tests for search and show are broken because > notmuch attempts to dereference a NULL pointer. > --- > This version fixes the "Show: text" test so that it sanitize its > output and doesn't hard-code my test paths. +1 Tomi w/ python json(.tool) the original order cannot be preserved as the parser stores content into dictionary -- without sorting those came in some internal python order... the following code could be used to use less indentation, though: python -c 'import sys,json; j = json.load(sys.stdin); json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file The other "problem" with json.tool is that it converts non-ascii chars to \u values :/. What we could do is to dig a simple c json formatter -- someday in the future, maybe -- but for *now* this is the best we can have :D > > test/missing-headers | 162 > ++ > test/notmuch-test|1 + > 2 files changed, 163 insertions(+) > create mode 100755 test/missing-headers > > diff --git a/test/missing-headers b/test/missing-headers > new file mode 100755 > index 000..e79f922 > --- /dev/null > +++ b/test/missing-headers > @@ -0,0 +1,162 @@ > +#!/usr/bin/env bash > +test_description='messages with missing headers' > +. ./test-lib.sh > + > +# Notmuch requires at least one of from, subject, or to or it will > +# ignore the file. Generate two messages so that together they cover > +# all possible missing headers. We also give one of the messages a > +# date to ensure stable result ordering. > + > +cat < "${MAIL_DIR}/msg-2" > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > + > +Body > +EOF > + > +cat < "${MAIL_DIR}/msg-1" > +From: Notmuch Test Suite > + > +Body > +EOF > + > +NOTMUCH_NEW > + > +test_begin_subtest "Search: text" > +output=$(notmuch search '*' | notmuch_search_sanitize) > +test_expect_equal "$output" "\ > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > + > +test_begin_subtest "Search: json" > +test_subtest_known_broken > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > +test_expect_equal_json "$output" ' > +[ > +{ > +"authors": "", > +"date_relative": "2001-01-05", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 978709437, > +"total": 1 > +}, > +{ > +"authors": "Notmuch Test Suite", > +"date_relative": "1970-01-01", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 0, > +"total": 1 > +} > +]' > + > +test_begin_subtest "Show: text" > +output=$(notmuch show '*' | notmuch_show_sanitize) > +test_expect_equal "$output" "\ > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-2 > +header{ > + (2001-01-05) (inbox unread) > +Subject: (null) > +From: (null) > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message} > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-1 > +header{ > +Notmuch Test Suite (1970-01-01) (inbox > unread) > +Subject: (null) > +From: Notmuch Test Suite > +Date: Thu, 01 Jan 1970 00:00:00 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message}" > + > +test_begin_subtest "Show: json" > +test_subtest_known_broken > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > +test_expect_equal_json "$output" ' > +[ > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "2001-01-05", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Fri, 05 Jan 2001 15:43:57 +", > +"From": "", > +"Subject": "", > +"To": "Notmuch Test Suite notmuchmail.org>" > +}, > +"id": "X", > +"match": true, > +"tags": [ > +"inbox", > +"unread" > +], > +"timestamp": 978709437 > +}, > +[] > +] > +], > +[ > +[ > +{ > +"body": [ > +
Re: [PATCH 3/3] test: add broken roundtrip test
On Wed, Aug 08 2012, Mark Walters wrote: >> test/dump-restore |9 + >> 1 file changed, 9 insertions(+) >> >> diff --git a/test/dump-restore b/test/dump-restore >> index 439e998..7979ebf 100755 >> --- a/test/dump-restore >> +++ b/test/dump-restore >> @@ -82,4 +82,13 @@ test_begin_subtest "dump outfile -- from:cworth" >> notmuch dump dump-outfile-dash-inbox.actual -- from:cworth >> test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual >> >> +test_expect_success 'roundtripping random message-ids and tags' \ >> +'test_subtest_known_broken && >> +${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} && >> +notmuch dump > EXPECTED.$test_count && >> +notmuch tag -random-corpus tag:random-corpus && >> +notmuch restore < EXPECTED.$test_count 2>/dev/null && >> +notmuch dump > OUTPUT.$test_count && >> +test_cmp EXPECTED.$test_count OUTPUT.$test_count 1>/dev/null' > > Are the single quotes at the start and end of the main block meant to be > there? And with them deleted this seems to pass (but there is lots of > diff if the redirection is removed). I am not familiar with > test_expect_success/test_cmp so don't know what to expect. I don't understand what's going on here either. This seems like a strange way to run these tests, as a command string to test_expect_success. Why not just run them directly? I'm also worried about the test output blowing away the users terminal. I think that should be avoided, even if we expect failures to be rare. jamie. pgpsbg0gK1xU5.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] test: add broken roundtrip test
On Wed, Aug 08 2012, Mark Walters wrote: >> test/dump-restore |9 + >> 1 file changed, 9 insertions(+) >> >> diff --git a/test/dump-restore b/test/dump-restore >> index 439e998..7979ebf 100755 >> --- a/test/dump-restore >> +++ b/test/dump-restore >> @@ -82,4 +82,13 @@ test_begin_subtest "dump outfile -- from:cworth" >> notmuch dump dump-outfile-dash-inbox.actual -- from:cworth >> test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual >> >> +test_expect_success 'roundtripping random message-ids and tags' \ >> +'test_subtest_known_broken && >> +${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} && >> +notmuch dump > EXPECTED.$test_count && >> +notmuch tag -random-corpus tag:random-corpus && >> +notmuch restore < EXPECTED.$test_count 2>/dev/null && >> +notmuch dump > OUTPUT.$test_count && >> +test_cmp EXPECTED.$test_count OUTPUT.$test_count 1>/dev/null' > > Are the single quotes at the start and end of the main block meant to be > there? And with them deleted this seems to pass (but there is lots of > diff if the redirection is removed). I am not familiar with > test_expect_success/test_cmp so don't know what to expect. I don't understand what's going on here either. This seems like a strange way to run these tests, as a command string to test_expect_success. Why not just run them directly? I'm also worried about the test output blowing away the users terminal. I think that should be avoided, even if we expect failures to be rare. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120808/8d637917/attachment.pgp>
Re: [PATCH] sprinters: bugfix when NULL passed for a string.
On Wed, Aug 08 2012, Mark Walters wrote: > For the From/Date headers see http://www.ietf.org/rfc/rfc2822.txt > section 3.6 for details: > >The only required header fields are the origination date field and >the originator address field(s). All other header fields are >syntactically optional. > > and origination date field means a Date: header, and originator > address field means a From: header. However, I don't think an empty > string is valid for either of these so we can't sensibly output > something standards compliant. Thus I would go for following the > original message and omit any fields not present in it. I agree. Let's not pretend or imply something is valid when it's not. The output should reflect the actual content of the message as much as possible. jamie. pgp8Tb3i46WCX.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] sprinters: bugfix when NULL passed for a string.
On Wed, Aug 08 2012, Mark Walters wrote: > For the From/Date headers see http://www.ietf.org/rfc/rfc2822.txt > section 3.6 for details: > >The only required header fields are the origination date field and >the originator address field(s). All other header fields are >syntactically optional. > > and origination date field means a Date: header, and originator > address field means a From: header. However, I don't think an empty > string is valid for either of these so we can't sensibly output > something standards compliant. Thus I would go for following the > original message and omit any fields not present in it. I agree. Let's not pretend or imply something is valid when it's not. The output should reflect the actual content of the message as much as possible. jamie. -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 835 bytes Desc: not available URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20120808/b504e295/attachment.pgp>
[PATCH 3/3] test: add broken roundtrip test
On Sun, 05 Aug 2012, david at tethera.net wrote: > From: David Bremner > > The output of test_cmp is redirected because it is pretty horrible, > and tends to mess up terminals. When the test is no longer marked as > broken, this redirection should be removed. How about piping the output to hexdump or even cat -v or something? > --- > test/dump-restore |9 + > 1 file changed, 9 insertions(+) > > diff --git a/test/dump-restore b/test/dump-restore > index 439e998..7979ebf 100755 > --- a/test/dump-restore > +++ b/test/dump-restore > @@ -82,4 +82,13 @@ test_begin_subtest "dump outfile -- from:cworth" > notmuch dump dump-outfile-dash-inbox.actual -- from:cworth > test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual > > +test_expect_success 'roundtripping random message-ids and tags' \ > +'test_subtest_known_broken && > +${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} && > +notmuch dump > EXPECTED.$test_count && > +notmuch tag -random-corpus tag:random-corpus && > +notmuch restore < EXPECTED.$test_count 2>/dev/null && > +notmuch dump > OUTPUT.$test_count && > +test_cmp EXPECTED.$test_count OUTPUT.$test_count 1>/dev/null' Are the single quotes at the start and end of the main block meant to be there? And with them deleted this seems to pass (but there is lots of diff if the redirection is removed). I am not familiar with test_expect_success/test_cmp so don't know what to expect. Best wishes Mark > + > test_done > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2] emacs: notmuch search bugfix
On Tue, Aug 07 2012, Mark Walters wrote: > The recent change to use json for notmuch-search.el introduced a bug > in the code for keeping position on refresh. The problem is a > comparison between (plist-get result :thread) and a thread-id returned > by notmuch-search-find-thread-id: the latter is prefixed with > "thread:" > > We fix this by adding an option to notmuch-search-find-thread-id to > return the bare thread-id. It appears that notmuch-search-refresh-view > is the only caller of notmuch-search that supplies a thread-id so this > change should be safe (but could theoretically break users .emacs > functions). > --- LGTM. Tomi > > I think this implements all of Austin's suggestions, and it adds a short > remark to NEWS (since in unlikely cases it could break users' functions) > and updates the docstring for notmuch-search. > > Best wishes > > Mark > > NEWS |3 +++ > emacs/notmuch.el | 12 +++- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/NEWS b/NEWS > index 761b2c1..9916c91 100644 > --- a/NEWS > +++ b/NEWS > @@ -55,6 +55,9 @@ user-specified formatting >you've customized this variable, you may have to change your date >format from `"%s "` to `"%12s "`. > > +The thread-id for the `target-thread` argument for `notmuch-search` should > +now be supplied without the "thread:" prefix. > + > Notmuch 0.13.2 (2012-06-02) > === > > diff --git a/emacs/notmuch.el b/emacs/notmuch.el > index d2d82a9..7b61e9b 100644 > --- a/emacs/notmuch.el > +++ b/emacs/notmuch.el > @@ -475,10 +475,12 @@ BEG." > (push (plist-get (notmuch-search-get-result pos) property) output))) > output)) > > -(defun notmuch-search-find-thread-id () > - "Return the thread for the current thread" > +(defun notmuch-search-find-thread-id (&optional bare) > + "Return the thread for the current thread > + > +If BARE is set then do not prefix with \"thread:\"" >(let ((thread (plist-get (notmuch-search-get-result) :thread))) > -(when thread (concat "thread:" thread > +(when thread (concat (unless bare "thread:") thread > > (defun notmuch-search-find-thread-id-region (beg end) >"Return a list of threads for the current region" > @@ -936,7 +938,7 @@ If `query' is nil, it is read interactively from the > minibuffer. > Other optional parameters are used as follows: > >oldest-first: A Boolean controlling the sort order of returned threads > - target-thread: A thread ID (with the thread: prefix) that will be made > + target-thread: A thread ID (without the thread: prefix) that will be made > current if it appears in the search results. >target-line: The line number to move to if the target thread does not > appear in the search results." > @@ -993,7 +995,7 @@ same relative position within the new buffer." >(interactive) >(let ((target-line (line-number-at-pos)) > (oldest-first notmuch-search-oldest-first) > - (target-thread (notmuch-search-find-thread-id)) > + (target-thread (notmuch-search-find-thread-id 'bare)) > (query notmuch-search-query-string) > (continuation notmuch-search-continuation)) > (notmuch-kill-this-buffer) > -- > 1.7.9.1 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] test: Add test for messages with missing headers
Quoth Tomi Ollila on Aug 08 at 11:08 am: > On Wed, Aug 08 2012, Austin Clements wrote: > > > Currently the JSON tests for search and show are broken because > > notmuch attempts to dereference a NULL pointer. > > --- > > This version fixes the "Show: text" test so that it sanitize its > > output and doesn't hard-code my test paths. > > +1 > > Tomi > > w/ python json(.tool) the original order cannot be preserved as the parser > stores content into dictionary -- without sorting those came in some > internal python order... the following code could be used to use less To be fair, it can output them in original order, but doing so requires Python 2.7: python -c 'import sys,json,collections; json.dump(json.load(sys.stdin, object_pairs_hook=collections.OrderedDict), sys.stdout, indent=4)' > indentation, though: > > python -c 'import sys,json; j = json.load(sys.stdin); > json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file > > The other "problem" with json.tool is that it converts non-ascii chars > to \u values :/. This one doesn't require 2.7. python -c 'import sys,json,codecs; json.dump(json.load(sys.stdin), codecs.getwriter("utf8")(sys.stdout), indent=4, ensure_ascii=False)' Though I think that, for test canonicalization, \u is probably less error/locale prone. > What we could do is to dig a simple c json formatter -- someday in the > future, maybe -- but for *now* this is the best we can have :D Not another JSON parser/printer! > > > > test/missing-headers | 162 > > ++ > > test/notmuch-test|1 + > > 2 files changed, 163 insertions(+) > > create mode 100755 test/missing-headers > > > > diff --git a/test/missing-headers b/test/missing-headers > > new file mode 100755 > > index 000..e79f922 > > --- /dev/null > > +++ b/test/missing-headers > > @@ -0,0 +1,162 @@ > > +#!/usr/bin/env bash > > +test_description='messages with missing headers' > > +. ./test-lib.sh > > + > > +# Notmuch requires at least one of from, subject, or to or it will > > +# ignore the file. Generate two messages so that together they cover > > +# all possible missing headers. We also give one of the messages a > > +# date to ensure stable result ordering. > > + > > +cat < "${MAIL_DIR}/msg-2" > > +To: Notmuch Test Suite > > +Date: Fri, 05 Jan 2001 15:43:57 + > > + > > +Body > > +EOF > > + > > +cat < "${MAIL_DIR}/msg-1" > > +From: Notmuch Test Suite > > + > > +Body > > +EOF > > + > > +NOTMUCH_NEW > > + > > +test_begin_subtest "Search: text" > > +output=$(notmuch search '*' | notmuch_search_sanitize) > > +test_expect_equal "$output" "\ > > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > > + > > +test_begin_subtest "Search: json" > > +test_subtest_known_broken > > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > > +test_expect_equal_json "$output" ' > > +[ > > +{ > > +"authors": "", > > +"date_relative": "2001-01-05", > > +"matched": 1, > > +"subject": "", > > +"tags": [ > > +"inbox", > > +"unread" > > +], > > +"thread": "XXX", > > +"timestamp": 978709437, > > +"total": 1 > > +}, > > +{ > > +"authors": "Notmuch Test Suite", > > +"date_relative": "1970-01-01", > > +"matched": 1, > > +"subject": "", > > +"tags": [ > > +"inbox", > > +"unread" > > +], > > +"thread": "XXX", > > +"timestamp": 0, > > +"total": 1 > > +} > > +]' > > + > > +test_begin_subtest "Show: text" > > +output=$(notmuch show '*' | notmuch_show_sanitize) > > +test_expect_equal "$output" "\ > > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > > match:1 excluded:0 filename:/XXX/mail/msg-2 > > +header{ > > + (2001-01-05) (inbox unread) > > +Subject: (null) > > +From: (null) > > +To: Notmuch Test Suite > > +Date: Fri, 05 Jan 2001 15:43:57 + > > +header} > > +body{ > > +part{ ID: 1, Content-type: text/plain > > +Body > > +part} > > +body} > > +message} > > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > > match:1 excluded:0 filename:/XXX/mail/msg-1 > > +header{ > > +Notmuch Test Suite (1970-01-01) (inbox unread) > > +Subject: (null) > > +From: Notmuch Test Suite > > +Date: Thu, 01 Jan 1970 00:00:00 + > > +header} > > +body{ > > +part{ ID: 1, Content-type: text/plain > > +Body > > +part} > > +body} > > +message}" > > + > > +test_begin_subtest "Show: json" > > +test_subtest_known_broken > > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > > +test_expect_equal_json "$output" ' > > +[ > > +[ > > +[ > > +{ > > +"body": [ > > +{ > > +"content": "Body\n", > > +"content
[PATCH 2/3] test: add generator for random "stub" messages
Hi I don't think I know enough Xapian to sensibly review the first patch in this series. On Sun, 05 Aug 2012, david at tethera.net wrote: > From: David Bremner > > Initial use case is testing dump and restore, so we only have > message-ids and tags. > > The message ID's are nothing like RFC compliant, but it doesn't seem > any harder to roundtrip random UTF-8 strings than RFC-compliant ones. > > Tags are UTF-8, even though notmuch is in principle more generous than > that. > --- > test/.gitignore |1 + > test/Makefile.local | 14 +++- > test/basic |2 +- > test/random-corpus.c | 201 > ++ > 4 files changed, 216 insertions(+), 2 deletions(-) > create mode 100644 test/random-corpus.c > > diff --git a/test/.gitignore b/test/.gitignore > index e63c689..e23017b 100644 > --- a/test/.gitignore > +++ b/test/.gitignore > @@ -3,4 +3,5 @@ corpus.mail > smtp-dummy > symbol-test > arg-test > +random-corpus > tmp.* > diff --git a/test/Makefile.local b/test/Makefile.local > index c7f1435..586efc6 100644 > --- a/test/Makefile.local > +++ b/test/Makefile.local > @@ -13,6 +13,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o) > $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a > $(call quiet,CC) -I. $^ -o $@ > > +random_corpus_deps = $(dir)/random-corpus.o $(dir)/database-test.o \ > + notmuch-config.o command-line-arguments.o \ > + lib/libnotmuch.a util/libutil.a > + > +$(dir)/random-corpus: $(random_corpus_deps) > + $(call quiet,CC) $(CFLAGS_FINAL) $^ -o $@ $(CONFIGURE_LDFLAGS) > + > $(dir)/smtp-dummy: $(smtp_dummy_modules) > $(call quiet,CC) $^ -o $@ > > @@ -21,7 +28,12 @@ $(dir)/symbol-test: $(dir)/symbol-test.o > > .PHONY: test check > > -test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test > +TEST_BINARIES=$(dir)/arg-test \ > + $(dir)/random-corpus \ > + $(dir)/smtp-dummy \ > + $(dir)/symbol-test > + > +test-binaries: $(TEST_BINARIES) > > test:all test-binaries > @${dir}/notmuch-test $(OPTIONS) > diff --git a/test/basic b/test/basic > index d6aed24..589c4e2 100755 > --- a/test/basic > +++ b/test/basic > @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be > run by notmuch-test' > eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test) > tests_in_suite=$(for i in $TESTS; do echo $i; done | sort) > available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf > '%f\n' | \ > -sed -r -e > "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" > | \ > +sed -r -e > "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test|random-corpus)$/d" > | \ > sort) > test_expect_equal "$tests_in_suite" "$available" > > diff --git a/test/random-corpus.c b/test/random-corpus.c > new file mode 100644 > index 000..ae900a6 > --- /dev/null > +++ b/test/random-corpus.c > @@ -0,0 +1,201 @@ > +/* > + * Generate a random corpus of stub messages. > + * > + * Initial use case is testing dump and restore, so we only have > + * message-ids and tags. > + * > + * Generated message-id's and tags are intentionally nasty. > + * > + * Copyright (c) 2012 David Bremner > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: David Bremner > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "notmuch-client.h" > +#include "command-line-arguments.h" > +#include "database-test.h" > + > +/* Current largest UTF-32 value defined. Note that most of these will > + * be printed as boxes in most fonts. > + */ > + > +#define GLYPH_MAX 0x10FFFE > + > +static gunichar > +random_unichar () > +{ > +int start=1, stop=GLYPH_MAX; > +int class = random() % 4; > + > +switch (class) { > +case 0: > + /* control */ > + start=0x01; > + stop=0x20; > + break; > +case 1: > + start=0x21; > + stop=0x7E; > + break; > +case 2: > + start=0x41; > + stop=0x7a; > + break; > +case 3: > + start=0x7F; > + stop=GLYPH_MAX; > +} I think comments on other the classes might be helpful: I think Case 2 is Ascii A-z but that has a few characters between Z and a.
[PATCH v2] test: Add test for messages with missing headers
On Wed, 08 Aug 2012, Austin Clements wrote: > Currently the JSON tests for search and show are broken because > notmuch attempts to dereference a NULL pointer. > --- > This version fixes the "Show: text" test so that it sanitize its > output and doesn't hard-code my test paths. LGTM +1. Tests work as expected and all tests pass with id:"874noe1o0r.fsf at qmul.ac.uk" applied. Best wishes Mark > test/missing-headers | 162 > ++ > test/notmuch-test|1 + > 2 files changed, 163 insertions(+) > create mode 100755 test/missing-headers > > diff --git a/test/missing-headers b/test/missing-headers > new file mode 100755 > index 000..e79f922 > --- /dev/null > +++ b/test/missing-headers > @@ -0,0 +1,162 @@ > +#!/usr/bin/env bash > +test_description='messages with missing headers' > +. ./test-lib.sh > + > +# Notmuch requires at least one of from, subject, or to or it will > +# ignore the file. Generate two messages so that together they cover > +# all possible missing headers. We also give one of the messages a > +# date to ensure stable result ordering. > + > +cat < "${MAIL_DIR}/msg-2" > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > + > +Body > +EOF > + > +cat < "${MAIL_DIR}/msg-1" > +From: Notmuch Test Suite > + > +Body > +EOF > + > +NOTMUCH_NEW > + > +test_begin_subtest "Search: text" > +output=$(notmuch search '*' | notmuch_search_sanitize) > +test_expect_equal "$output" "\ > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > + > +test_begin_subtest "Search: json" > +test_subtest_known_broken > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > +test_expect_equal_json "$output" ' > +[ > +{ > +"authors": "", > +"date_relative": "2001-01-05", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 978709437, > +"total": 1 > +}, > +{ > +"authors": "Notmuch Test Suite", > +"date_relative": "1970-01-01", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 0, > +"total": 1 > +} > +]' > + > +test_begin_subtest "Show: text" > +output=$(notmuch show '*' | notmuch_show_sanitize) > +test_expect_equal "$output" "\ > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-2 > +header{ > + (2001-01-05) (inbox unread) > +Subject: (null) > +From: (null) > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message} > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-1 > +header{ > +Notmuch Test Suite (1970-01-01) (inbox > unread) > +Subject: (null) > +From: Notmuch Test Suite > +Date: Thu, 01 Jan 1970 00:00:00 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message}" > + > +test_begin_subtest "Show: json" > +test_subtest_known_broken > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > +test_expect_equal_json "$output" ' > +[ > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "2001-01-05", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Fri, 05 Jan 2001 15:43:57 +", > +"From": "", > +"Subject": "", > +"To": "Notmuch Test Suite notmuchmail.org>" > +}, > +"id": "X", > +"match": true, > +"tags": [ > +"inbox", > +"unread" > +], > +"timestamp": 978709437 > +}, > +[] > +] > +], > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "1970-01-01", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Thu, 01 Jan 1970 00:00:00 +", > +"From": "Notmuch Test
[PATCH] sprinters: bugfix when NULL passed for a string.
On Wed, 08 Aug 2012, Austin Clements wrote: > LGTM. > > This won't commute with [0], since that introduces broken tests that are > fixed by this patch. Yes: I guess the tidiest might be for me to send a version of my patch which marks these tests fixed so it can be applied after this. > I think we should remove the fields in the JSON header object for > missing headers (except perhaps From and Date, if those really are > mandatory headers), but I think we should do that after the freeze, > since it might affect frontends. I agree with you and Jamie on this. I think it is a `feature' rather than a bugfix (the current patch just restores the output to what it was before the sprinter changes it) so agree it should be after the freeze. We could deprecate passing NULL to sprinter string functions (possibly keep the check but fprintf a warning?) For the From/Date headers see http://www.ietf.org/rfc/rfc2822.txt section 3.6 for details: The only required header fields are the origination date field and the originator address field(s). All other header fields are syntactically optional. and origination date field means a Date: header, and originator address field means a From: header. However, I don't think an empty string is valid for either of these so we can't sensibly output something standards compliant. Thus I would go for following the original message and omit any fields not present in it. > It probably makes sense to add an Emacs test or two to the tests in [0]. This seems like a good idea. Best wishes Mark > [0] id:"1344389313-7886-1-git-send-email-amdragon at mit.edu" > > On Tue, 07 Aug 2012, Mark Walters wrote: >> The string function in a sprinter may be called with a NULL string >> pointer (eg if a header is absent). This causes a segfault. We fix >> this by checking for a null pointer in the string functions and update >> the sprinter documentation. >> >> At the moment some output when format=text is done directly rather than >> via an sprinter: in that case a null pointer is passed to printf or >> similar and a "(null)" appears in the output. That behaviour is not >> changed in this patch. >> --- >> >> This could really do with some tests (it is the second time this type of >> bug has occurred). To be considered as a message by notmuch new a file >> needs at least one of a From: Subject: or To: header. Thus we should >> have three messages each of which just contains that single header (and >> nothing else) and check that search and show work as expected. >> >> >> >> sprinter-json.c |2 ++ >> sprinter-text.c |2 ++ >> sprinter.h |4 +++- >> 3 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/sprinter-json.c b/sprinter-json.c >> index c9b6835..0a07790 100644 >> --- a/sprinter-json.c >> +++ b/sprinter-json.c >> @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, >> size_t len) >> static void >> json_string (struct sprinter *sp, const char *val) >> { >> +if (val == NULL) >> +val = ""; >> json_string_len (sp, val, strlen (val)); >> } >> >> diff --git a/sprinter-text.c b/sprinter-text.c >> index dfa54b5..10343be 100644 >> --- a/sprinter-text.c >> +++ b/sprinter-text.c >> @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, >> size_t len) >> static void >> text_string (struct sprinter *sp, const char *val) >> { >> +if (val == NULL) >> +val = ""; >> text_string_len (sp, val, strlen (val)); >> } >> >> diff --git a/sprinter.h b/sprinter.h >> index 5f43175..912a526 100644 >> --- a/sprinter.h >> +++ b/sprinter.h >> @@ -27,7 +27,9 @@ typedef struct sprinter { >> * a list or map, followed or preceded by separators). For string >> * and string_len, the char * must be UTF-8 encoded. string_len >> * allows non-terminated strings and strings with embedded NULs >> - * (though the handling of the latter is format-dependent). >> + * (though the handling of the latter is format-dependent). For >> + * string (but not string_len) the string pointer passed may be >> + * NULL. >> */ >> void (*string) (struct sprinter *, const char *); >> void (*string_len) (struct sprinter *, const char *, size_t); >> -- >> 1.7.9.1 >> >> >> H >> ___ >> notmuch mailing list >> notmuch at notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 1/4] show: indicate length of omitted body content (json)
On Wed, 08 Aug 2012 00:01:06 +0100, Mark Walters wrote: > > I have two minor queries: do you think omitted text/html parts should > also have the length given? Or even should it always be sent? But I am > happy with it as is. It could be added. I wouldn't have much use for it myself. text/html parts should usually be small enough that it doesn't matter, and I never retrieve them when there is a text/plain alternative. Peter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/3] test: conform to content length, encoding fields
Update tests to expect content-length and content-transfer-encoding fields in show --format=json output, for leaf parts with omitted body content. --- test/crypto| 30 +- test/json |2 +- test/multipart |9 + 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/test/crypto b/test/crypto index 5dd14c4..aa96ec2 100755 --- a/test/crypto +++ b/test/crypto @@ -61,7 +61,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -95,7 +96,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -127,7 +129,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ @@ -196,7 +199,8 @@ expected='[[[{"id": "X", "sigstatus": [], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, "content-type": "multipart/mixed", "content": [{"id": 4, @@ -204,6 +208,8 @@ expected='[[[{"id": "X", "content": "This is a test encrypted message.\n"}, {"id": 5, "content-type": "application/octet-stream", + "content-length": 28, + "content-transfer-encoding": "base64", "filename": "TESTATTACHMENT"}]}]}]}, [' test_expect_equal_json \ @@ -231,9 +237,11 @@ test_expect_equal_file OUTPUT TESTATTACHMENT test_begin_subtest "decryption failure with missing key" mv "${GNUPGHOME}"{,.bak} +# The length of the encrypted attachment varies so must be normalized. output=$(notmuch show --format=json --decrypt subject:"test encrypted message 001" \ | notmuch_json_show_sanitize \ -| sed -e 's|"created": [1234567890]*|"created": 946728000|') +| sed -e 's|"created": [1234567890]*|"created": 946728000|' \ +| sed -e 's|"content-length": 6[1234567890]*|"content-length": 652|') expected='[[[{"id": "X", "match": true, "excluded": false, @@ -249,9 +257,11 @@ expected='[[[{"id": "X", "encstatus": [{"status": "bad"}], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, - "content-type": "application/octet-stream"}]}]}, + "content-type": "application/octet-stream", + "content-length": 652}]}]}, [' test_expect_equal_json \ "$output" \ @@ -287,7 +297,8 @@ expected='[[[{"id": "X", "userid": " Notmuch Test Suite (INSECURE!)"}], "content-type": "multipart/encrypted", "content": [{"id": 2, - "content-type": "application/pgp-encrypted"}, + "content-type": "application/pgp-encrypted", + "content-length": 11}, {"id": 3, "content-type": "text/plain", "content": "This is another test encrypted message.\n"}]}]}, @@ -342,7 +353,8 @@ expected='[[[{"id": "X", "content-type": "text/plain", "content": "This is a test signed message.\n"}, {"id": 3, - "content-type": "application/pgp-signature"}]}]}, + "content-type": "application/pgp-signature", + "content-length": 315}]}]}, [' test_expect_equal_json \ "$output" \ diff --git a/test/json b/test/json index ac8fa8e..8ce2e8a 100755 --- a/test/json +++ b/test/json @@ -45,7 +45,7 @@ emacs_deliver_message \ (insert \"Message-ID: <$id>\n\")" output=$(notmuch show --format=json "id:$id") filename=$(notmuch search --output=files "id:$id") -test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\"Subject\": \"$subject\", \"From\": \"Notmuch Test Suite \", \"To\": \"test_su...@notmuchmail.org\", \"Date\": \"Sat, 01 Jan 2000 12:00:00 +\"}, \"body\": [{\"id\": 1, \"content-type\": \"multipart/mixed\", \"content\": [{\"id\": 2, \"content-type\": \"text/plain\", \"content\": \"This is a test message with inline attachment with a filename\"}, {\"id\": 3, \"content-type\": \"application/octet-stream\", \"filename\": \"README\"}]}]}, [" +test_expect_equal_json "$output" "[[[{\"id\": \"$id\", \"match\": true, \"excluded\": false, \"filename\": \"$filename\", \"timestamp\": 946728000, \"date_relative\": \"2000-01-01\", \"tags\": [\"inbox\"], \"headers\": {\
[PATCH v2 2/3] show: indicate length, encoding of omitted body content
If a leaf part's body content is omitted, return the encoded length and transfer encoding in --format=json output. This information may be used by the consumer, e.g. to decide whether to download a large attachment over a slow link. Returning the _encoded_ content length is more efficient than returning the _decoded_ content length. Returning the transfer encoding allows the consumer to estimate the decoded content length. --- devel/schemata |9 - notmuch-show.c | 14 ++ 2 files changed, 22 insertions(+), 1 deletions(-) diff --git a/devel/schemata b/devel/schemata index 9cb25f5..094b9a5 100644 --- a/devel/schemata +++ b/devel/schemata @@ -69,7 +69,14 @@ part = { # A leaf part's body content is optional, but may be included if # it can be correctly encoded as a string. Consumers should use # this in preference to fetching the part content separately. -content?: string +content?: string, +# If a leaf part's body content is not included, the length of +# the encoded content (in bytes) may be given instead. +content-length?: int, +# If a leaf part's body content is not included, its transfer encoding +# may be given. Using this and the encoded content length, it is +# possible for the consumer to estimate the decoded content length. +content-transfer-encoding?: string } # The headers of a message or part (format_headers_json with reply = FALSE) diff --git a/notmuch-show.c b/notmuch-show.c index 3556293..83535c7 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -664,6 +664,20 @@ format_part_json (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->map_key (sp, "content"); sp->string_len (sp, (char *) part_content->data, part_content->len); g_object_unref (stream_memory); + } else { + GMimeDataWrapper *wrapper = g_mime_part_get_content_object (GMIME_PART (node->part)); + GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper); + ssize_t length = g_mime_stream_length (stream); + const char *cte = g_mime_object_get_header (meta, "content-transfer-encoding"); + + if (length >= 0) { + sp->map_key (sp, "content-length"); + sp->integer (sp, length); + } + if (cte) { + sp->map_key (sp, "content-transfer-encoding"); + sp->string (sp, cte); + } } } else if (GMIME_IS_MULTIPART (node->part)) { sp->map_key (sp, "content"); -- 1.7.4.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 1/3] test: normalize only message filenames in show json
notmuch_json_show_sanitize replaced "filename" field values even in part structures, where the value is predictable. Make it only normalize the filename value if it is an absolute path (begins with slash), which is true of the Maildir filenames that were intended to be normalized away. --- test/multipart |2 +- test/test-lib.sh |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/multipart b/test/multipart index 0527f84..344ed81 100755 --- a/test/multipart +++ b/test/multipart @@ -630,7 +630,7 @@ cat
[PATCH v2 0/3] indicate length of omitted body content
The first commit is tangentally related. An expected test case output in test/crypto previously had a filename left unnormalised by notmuch_json_show_sanitize because it was not followed by comma. The next commit causes the comma to be present, breaking the expected output. In the 2nd commit, a content-transfer-encoding field was added and comments clarified. In the 3rd commit, the content-length of an encrypted attachment had to be normalised because it varies between runs. Peter Wang (3): test: normalize only message filenames in show json show: indicate length, encoding of omitted body content test: conform to content length, encoding fields devel/schemata |9 - notmuch-show.c | 14 ++ test/crypto | 30 +- test/json|2 +- test/multipart | 11 ++- test/test-lib.sh |2 +- 6 files changed, 51 insertions(+), 17 deletions(-) -- 1.7.4.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2] test: Add test for messages with missing headers
On Wed, Aug 08 2012, Austin Clements wrote: > Currently the JSON tests for search and show are broken because > notmuch attempts to dereference a NULL pointer. > --- > This version fixes the "Show: text" test so that it sanitize its > output and doesn't hard-code my test paths. +1 Tomi w/ python json(.tool) the original order cannot be preserved as the parser stores content into dictionary -- without sorting those came in some internal python order... the following code could be used to use less indentation, though: python -c 'import sys,json; j = json.load(sys.stdin); json.dump(j, sys.stdout, sort_keys=True, indent=2)' < input_file The other "problem" with json.tool is that it converts non-ascii chars to \u values :/. What we could do is to dig a simple c json formatter -- someday in the future, maybe -- but for *now* this is the best we can have :D > > test/missing-headers | 162 > ++ > test/notmuch-test|1 + > 2 files changed, 163 insertions(+) > create mode 100755 test/missing-headers > > diff --git a/test/missing-headers b/test/missing-headers > new file mode 100755 > index 000..e79f922 > --- /dev/null > +++ b/test/missing-headers > @@ -0,0 +1,162 @@ > +#!/usr/bin/env bash > +test_description='messages with missing headers' > +. ./test-lib.sh > + > +# Notmuch requires at least one of from, subject, or to or it will > +# ignore the file. Generate two messages so that together they cover > +# all possible missing headers. We also give one of the messages a > +# date to ensure stable result ordering. > + > +cat < "${MAIL_DIR}/msg-2" > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > + > +Body > +EOF > + > +cat < "${MAIL_DIR}/msg-1" > +From: Notmuch Test Suite > + > +Body > +EOF > + > +NOTMUCH_NEW > + > +test_begin_subtest "Search: text" > +output=$(notmuch search '*' | notmuch_search_sanitize) > +test_expect_equal "$output" "\ > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > + > +test_begin_subtest "Search: json" > +test_subtest_known_broken > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > +test_expect_equal_json "$output" ' > +[ > +{ > +"authors": "", > +"date_relative": "2001-01-05", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 978709437, > +"total": 1 > +}, > +{ > +"authors": "Notmuch Test Suite", > +"date_relative": "1970-01-01", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 0, > +"total": 1 > +} > +]' > + > +test_begin_subtest "Show: text" > +output=$(notmuch show '*' | notmuch_show_sanitize) > +test_expect_equal "$output" "\ > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-2 > +header{ > + (2001-01-05) (inbox unread) > +Subject: (null) > +From: (null) > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message} > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-1 > +header{ > +Notmuch Test Suite (1970-01-01) (inbox unread) > +Subject: (null) > +From: Notmuch Test Suite > +Date: Thu, 01 Jan 1970 00:00:00 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message}" > + > +test_begin_subtest "Show: json" > +test_subtest_known_broken > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > +test_expect_equal_json "$output" ' > +[ > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "2001-01-05", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Fri, 05 Jan 2001 15:43:57 +", > +"From": "", > +"Subject": "", > +"To": "Notmuch Test Suite " > +}, > +"id": "X", > +"match": true, > +"tags": [ > +"inbox", > +"unread" > +], > +"timestamp": 978709437 > +}, > +[] > +] > +], > +[ > +[ > +{ > +"body": [ > +{
Re: [PATCH 3/3] test: add broken roundtrip test
On Sun, 05 Aug 2012, da...@tethera.net wrote: > From: David Bremner > > The output of test_cmp is redirected because it is pretty horrible, > and tends to mess up terminals. When the test is no longer marked as > broken, this redirection should be removed. How about piping the output to hexdump or even cat -v or something? > --- > test/dump-restore |9 + > 1 file changed, 9 insertions(+) > > diff --git a/test/dump-restore b/test/dump-restore > index 439e998..7979ebf 100755 > --- a/test/dump-restore > +++ b/test/dump-restore > @@ -82,4 +82,13 @@ test_begin_subtest "dump outfile -- from:cworth" > notmuch dump dump-outfile-dash-inbox.actual -- from:cworth > test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual > > +test_expect_success 'roundtripping random message-ids and tags' \ > +'test_subtest_known_broken && > +${TEST_DIRECTORY}/random-corpus --config-path=${NOTMUCH_CONFIG} && > +notmuch dump > EXPECTED.$test_count && > +notmuch tag -random-corpus tag:random-corpus && > +notmuch restore < EXPECTED.$test_count 2>/dev/null && > +notmuch dump > OUTPUT.$test_count && > +test_cmp EXPECTED.$test_count OUTPUT.$test_count 1>/dev/null' Are the single quotes at the start and end of the main block meant to be there? And with them deleted this seems to pass (but there is lots of diff if the redirection is removed). I am not familiar with test_expect_success/test_cmp so don't know what to expect. Best wishes Mark > + > test_done > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 2/3] test: add generator for random "stub" messages
Hi I don't think I know enough Xapian to sensibly review the first patch in this series. On Sun, 05 Aug 2012, da...@tethera.net wrote: > From: David Bremner > > Initial use case is testing dump and restore, so we only have > message-ids and tags. > > The message ID's are nothing like RFC compliant, but it doesn't seem > any harder to roundtrip random UTF-8 strings than RFC-compliant ones. > > Tags are UTF-8, even though notmuch is in principle more generous than > that. > --- > test/.gitignore |1 + > test/Makefile.local | 14 +++- > test/basic |2 +- > test/random-corpus.c | 201 > ++ > 4 files changed, 216 insertions(+), 2 deletions(-) > create mode 100644 test/random-corpus.c > > diff --git a/test/.gitignore b/test/.gitignore > index e63c689..e23017b 100644 > --- a/test/.gitignore > +++ b/test/.gitignore > @@ -3,4 +3,5 @@ corpus.mail > smtp-dummy > symbol-test > arg-test > +random-corpus > tmp.* > diff --git a/test/Makefile.local b/test/Makefile.local > index c7f1435..586efc6 100644 > --- a/test/Makefile.local > +++ b/test/Makefile.local > @@ -13,6 +13,13 @@ smtp_dummy_modules = $(smtp_dummy_srcs:.c=.o) > $(dir)/arg-test: $(dir)/arg-test.o command-line-arguments.o util/libutil.a > $(call quiet,CC) -I. $^ -o $@ > > +random_corpus_deps = $(dir)/random-corpus.o $(dir)/database-test.o \ > + notmuch-config.o command-line-arguments.o \ > + lib/libnotmuch.a util/libutil.a > + > +$(dir)/random-corpus: $(random_corpus_deps) > + $(call quiet,CC) $(CFLAGS_FINAL) $^ -o $@ $(CONFIGURE_LDFLAGS) > + > $(dir)/smtp-dummy: $(smtp_dummy_modules) > $(call quiet,CC) $^ -o $@ > > @@ -21,7 +28,12 @@ $(dir)/symbol-test: $(dir)/symbol-test.o > > .PHONY: test check > > -test-binaries: $(dir)/arg-test $(dir)/smtp-dummy $(dir)/symbol-test > +TEST_BINARIES=$(dir)/arg-test \ > + $(dir)/random-corpus \ > + $(dir)/smtp-dummy \ > + $(dir)/symbol-test > + > +test-binaries: $(TEST_BINARIES) > > test:all test-binaries > @${dir}/notmuch-test $(OPTIONS) > diff --git a/test/basic b/test/basic > index d6aed24..589c4e2 100755 > --- a/test/basic > +++ b/test/basic > @@ -54,7 +54,7 @@ test_begin_subtest 'Ensure that all available tests will be > run by notmuch-test' > eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test) > tests_in_suite=$(for i in $TESTS; do echo $i; done | sort) > available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -executable -printf > '%f\n' | \ > -sed -r -e > "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test)$/d" > | \ > +sed -r -e > "/^(aggregate-results.sh|notmuch-test|smtp-dummy|test-verbose|symbol-test|arg-test|random-corpus)$/d" > | \ > sort) > test_expect_equal "$tests_in_suite" "$available" > > diff --git a/test/random-corpus.c b/test/random-corpus.c > new file mode 100644 > index 000..ae900a6 > --- /dev/null > +++ b/test/random-corpus.c > @@ -0,0 +1,201 @@ > +/* > + * Generate a random corpus of stub messages. > + * > + * Initial use case is testing dump and restore, so we only have > + * message-ids and tags. > + * > + * Generated message-id's and tags are intentionally nasty. > + * > + * Copyright (c) 2012 David Bremner > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: David Bremner > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "notmuch-client.h" > +#include "command-line-arguments.h" > +#include "database-test.h" > + > +/* Current largest UTF-32 value defined. Note that most of these will > + * be printed as boxes in most fonts. > + */ > + > +#define GLYPH_MAX 0x10FFFE > + > +static gunichar > +random_unichar () > +{ > +int start=1, stop=GLYPH_MAX; > +int class = random() % 4; > + > +switch (class) { > +case 0: > + /* control */ > + start=0x01; > + stop=0x20; > + break; > +case 1: > + start=0x21; > + stop=0x7E; > + break; > +case 2: > + start=0x41; > + stop=0x7a; > + break; > +case 3: > + start=0x7F; > + stop=GLYPH_MAX; > +} I think comments on other the classes might be helpful: I think Case 2 is Ascii A-z but that has a few characters between Z and a. Of
Re: [PATCH v2] test: Add test for messages with missing headers
On Wed, 08 Aug 2012, Austin Clements wrote: > Currently the JSON tests for search and show are broken because > notmuch attempts to dereference a NULL pointer. > --- > This version fixes the "Show: text" test so that it sanitize its > output and doesn't hard-code my test paths. LGTM +1. Tests work as expected and all tests pass with id:"874noe1o0r@qmul.ac.uk" applied. Best wishes Mark > test/missing-headers | 162 > ++ > test/notmuch-test|1 + > 2 files changed, 163 insertions(+) > create mode 100755 test/missing-headers > > diff --git a/test/missing-headers b/test/missing-headers > new file mode 100755 > index 000..e79f922 > --- /dev/null > +++ b/test/missing-headers > @@ -0,0 +1,162 @@ > +#!/usr/bin/env bash > +test_description='messages with missing headers' > +. ./test-lib.sh > + > +# Notmuch requires at least one of from, subject, or to or it will > +# ignore the file. Generate two messages so that together they cover > +# all possible missing headers. We also give one of the messages a > +# date to ensure stable result ordering. > + > +cat < "${MAIL_DIR}/msg-2" > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > + > +Body > +EOF > + > +cat < "${MAIL_DIR}/msg-1" > +From: Notmuch Test Suite > + > +Body > +EOF > + > +NOTMUCH_NEW > + > +test_begin_subtest "Search: text" > +output=$(notmuch search '*' | notmuch_search_sanitize) > +test_expect_equal "$output" "\ > +thread:XXX 2001-01-05 [1/1] (null); (inbox unread) > +thread:XXX 1970-01-01 [1/1] Notmuch Test Suite; (inbox unread)" > + > +test_begin_subtest "Search: json" > +test_subtest_known_broken > +output=$(notmuch search --format=json '*' | notmuch_search_sanitize) > +test_expect_equal_json "$output" ' > +[ > +{ > +"authors": "", > +"date_relative": "2001-01-05", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 978709437, > +"total": 1 > +}, > +{ > +"authors": "Notmuch Test Suite", > +"date_relative": "1970-01-01", > +"matched": 1, > +"subject": "", > +"tags": [ > +"inbox", > +"unread" > +], > +"thread": "XXX", > +"timestamp": 0, > +"total": 1 > +} > +]' > + > +test_begin_subtest "Show: text" > +output=$(notmuch show '*' | notmuch_show_sanitize) > +test_expect_equal "$output" "\ > +message{ id:notmuch-sha1-7a6e4eac383ef958fcd3ebf2143db71b8ff01161 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-2 > +header{ > + (2001-01-05) (inbox unread) > +Subject: (null) > +From: (null) > +To: Notmuch Test Suite > +Date: Fri, 05 Jan 2001 15:43:57 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message} > +message{ id:notmuch-sha1-ca55943aff7a72baf2ab21fa74fab3d632401334 depth:0 > match:1 excluded:0 filename:/XXX/mail/msg-1 > +header{ > +Notmuch Test Suite (1970-01-01) (inbox unread) > +Subject: (null) > +From: Notmuch Test Suite > +Date: Thu, 01 Jan 1970 00:00:00 + > +header} > +body{ > +part{ ID: 1, Content-type: text/plain > +Body > +part} > +body} > +message}" > + > +test_begin_subtest "Show: json" > +test_subtest_known_broken > +output=$(notmuch show --format=json '*' | notmuch_json_show_sanitize) > +test_expect_equal_json "$output" ' > +[ > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "2001-01-05", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Fri, 05 Jan 2001 15:43:57 +", > +"From": "", > +"Subject": "", > +"To": "Notmuch Test Suite " > +}, > +"id": "X", > +"match": true, > +"tags": [ > +"inbox", > +"unread" > +], > +"timestamp": 978709437 > +}, > +[] > +] > +], > +[ > +[ > +{ > +"body": [ > +{ > +"content": "Body\n", > +"content-type": "text/plain", > +"id": 1 > +} > +], > +"date_relative": "1970-01-01", > +"excluded": false, > +"filename": "Y", > +"headers": { > +"Date": "Thu, 01 Jan 1970 00:00:00 +", > +"From": "Notmuch Test Suite > ", > +
Re: [PATCH] sprinters: bugfix when NULL passed for a string.
On Wed, 08 Aug 2012, Austin Clements wrote: > LGTM. > > This won't commute with [0], since that introduces broken tests that are > fixed by this patch. Yes: I guess the tidiest might be for me to send a version of my patch which marks these tests fixed so it can be applied after this. > I think we should remove the fields in the JSON header object for > missing headers (except perhaps From and Date, if those really are > mandatory headers), but I think we should do that after the freeze, > since it might affect frontends. I agree with you and Jamie on this. I think it is a `feature' rather than a bugfix (the current patch just restores the output to what it was before the sprinter changes it) so agree it should be after the freeze. We could deprecate passing NULL to sprinter string functions (possibly keep the check but fprintf a warning?) For the From/Date headers see http://www.ietf.org/rfc/rfc2822.txt section 3.6 for details: The only required header fields are the origination date field and the originator address field(s). All other header fields are syntactically optional. and origination date field means a Date: header, and originator address field means a From: header. However, I don't think an empty string is valid for either of these so we can't sensibly output something standards compliant. Thus I would go for following the original message and omit any fields not present in it. > It probably makes sense to add an Emacs test or two to the tests in [0]. This seems like a good idea. Best wishes Mark > [0] id:"1344389313-7886-1-git-send-email-amdra...@mit.edu" > > On Tue, 07 Aug 2012, Mark Walters wrote: >> The string function in a sprinter may be called with a NULL string >> pointer (eg if a header is absent). This causes a segfault. We fix >> this by checking for a null pointer in the string functions and update >> the sprinter documentation. >> >> At the moment some output when format=text is done directly rather than >> via an sprinter: in that case a null pointer is passed to printf or >> similar and a "(null)" appears in the output. That behaviour is not >> changed in this patch. >> --- >> >> This could really do with some tests (it is the second time this type of >> bug has occurred). To be considered as a message by notmuch new a file >> needs at least one of a From: Subject: or To: header. Thus we should >> have three messages each of which just contains that single header (and >> nothing else) and check that search and show work as expected. >> >> >> >> sprinter-json.c |2 ++ >> sprinter-text.c |2 ++ >> sprinter.h |4 +++- >> 3 files changed, 7 insertions(+), 1 deletions(-) >> >> diff --git a/sprinter-json.c b/sprinter-json.c >> index c9b6835..0a07790 100644 >> --- a/sprinter-json.c >> +++ b/sprinter-json.c >> @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, >> size_t len) >> static void >> json_string (struct sprinter *sp, const char *val) >> { >> +if (val == NULL) >> +val = ""; >> json_string_len (sp, val, strlen (val)); >> } >> >> diff --git a/sprinter-text.c b/sprinter-text.c >> index dfa54b5..10343be 100644 >> --- a/sprinter-text.c >> +++ b/sprinter-text.c >> @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, >> size_t len) >> static void >> text_string (struct sprinter *sp, const char *val) >> { >> +if (val == NULL) >> +val = ""; >> text_string_len (sp, val, strlen (val)); >> } >> >> diff --git a/sprinter.h b/sprinter.h >> index 5f43175..912a526 100644 >> --- a/sprinter.h >> +++ b/sprinter.h >> @@ -27,7 +27,9 @@ typedef struct sprinter { >> * a list or map, followed or preceded by separators). For string >> * and string_len, the char * must be UTF-8 encoded. string_len >> * allows non-terminated strings and strings with embedded NULs >> - * (though the handling of the latter is format-dependent). >> + * (though the handling of the latter is format-dependent). For >> + * string (but not string_len) the string pointer passed may be >> + * NULL. >> */ >> void (*string) (struct sprinter *, const char *); >> void (*string_len) (struct sprinter *, const char *, size_t); >> -- >> 1.7.9.1 >> >> >> H >> ___ >> notmuch mailing list >> notmuch@notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/4] show: indicate length of omitted body content (json)
I like this (and agree with Austin and you that text and json can diverge). I just hacked something together which uses this and makes the emacs front-end display the content-length on part buttons and as someone who uses notmuch over ssh that is nice. I have two minor queries: do you think omitted text/html parts should also have the length given? Or even should it always be sent? But I am happy with it as is. I haven't checked the test patch (or the text ones as they are being dropped). Best wishes Mark On Sun, 05 Aug 2012, Peter Wang wrote: > If a leaf part's body content is omitted, return the content length in > --format=json output. This information may be used by the consumer, > e.g. to decide whether to download a large attachment over a slow link. > --- > devel/schemata |5 - > notmuch-show.c |8 > 2 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/devel/schemata b/devel/schemata > index 9cb25f5..3df2764 100644 > --- a/devel/schemata > +++ b/devel/schemata > @@ -69,7 +69,10 @@ part = { > # A leaf part's body content is optional, but may be included if > # it can be correctly encoded as a string. Consumers should use > # this in preference to fetching the part content separately. > -content?: string > +content?: string, > +# If a leaf part's body content is not included, the content-length > +# may be included instead. > +content-length?: int > } > > # The headers of a message or part (format_headers_json with reply = FALSE) > diff --git a/notmuch-show.c b/notmuch-show.c > index 3556293..5c54257 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -664,6 +664,14 @@ format_part_json (const void *ctx, sprinter_t *sp, > mime_node_t *node, > sp->map_key (sp, "content"); > sp->string_len (sp, (char *) part_content->data, part_content->len); > g_object_unref (stream_memory); > + } else { > + GMimeDataWrapper *wrapper = g_mime_part_get_content_object > (GMIME_PART (node->part)); > + GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper); > + ssize_t length = g_mime_stream_length (stream); > + if (length >= 0) { > + sp->map_key (sp, "content-length"); > + sp->integer (sp, length); > + } > } > } else if (GMIME_IS_MULTIPART (node->part)) { > sp->map_key (sp, "content"); > -- > 1.7.4.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch