[PATCH 2/2] test: add "notmuch-show for message with invalid From" test

2011-05-12 Thread Dmitry Kurochkin
---
 test/emacs |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 2e85b0c..f2983a7 100755
--- a/test/emacs
+++ b/test/emacs
@@ -37,8 +37,21 @@ output=$(test_emacs "(notmuch-show 
\"$maildir_storage_thread\") (princ (buffer-s
 expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
 test_expect_equal "$output" "$expected"

+test_begin_subtest "notmuch-show for message with invalid From"
+add_message "[subject]=\"message-with-invalid-from\"" "[from]=\"\\\"Invalid 
\\\" From\\\" \""
+thread=$(notmuch search --output=threads subject:message-with-invalid-from)
+output=$(test_emacs "(notmuch-show \"$thread\") (princ (buffer-string))")
+test_expect_equal "$output" \
+'"Invalid " From"  (2001-01-05) (inbox)
+Subject: message-with-invalid-from
+To: Notmuch Test Suite 
+Date: Tue, 05 Jan 2001 15:43:57 -
+
+This is just a test message (#1)'
+
 test_begin_subtest "Navigation of notmuch-search to thread view"
 output=$(test_emacs '(notmuch-search "tag:inbox") (notmuch-test-wait) 
(goto-char (point-min)) (re-search-forward "Working with Maildir") 
(notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))')
+expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
 test_expect_equal "$output" "$expected"

 test_begin_subtest "Add tag from search view"
-- 
1.7.5.1



[PATCH 1/2] Make `notmuch-show-clean-address' parsing-error-proof.

2011-05-12 Thread Dmitry Kurochkin
Mail-header-parse-address may fail for an invalid address.
Before the change, this would result in empty notmuch-show buffer
with an error message like: Scan error: "Unbalanced parentheses".
The patch wraps the function in condition-case and returns
unchanged address in case of error.
---
 emacs/notmuch-show.el |   35 +++
 1 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 23291ce..bd348e1 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -224,22 +224,25 @@ same as that of the previous message."
 ")"))

 (defun notmuch-show-clean-address (address)
-  "Clean a single email address for display."
-  (let* ((parsed (mail-header-parse-address address))
-(address (car parsed))
-(name (cdr parsed)))
-;; Remove double quotes. They might be required during transport,
-;; but we don't need to see them.
-(when name
-  (setq name (replace-regexp-in-string "\"" "" name)))
-;; If the address is 'foo at bar.com ' then show just
-;; 'foo at bar.com'.
-(when (string= name address)
-  (setq name nil))
-
-(if (not name)
-   address
-  (concat name " <" address ">"
+  "Try to clean a single email ADDRESS for display.  Return
+unchanged ADDRESS if parsing fails."
+  (condition-case nil
+(let* ((parsed (mail-header-parse-address address))
+  (address (car parsed))
+  (name (cdr parsed)))
+  ;; Remove double quotes. They might be required during transport,
+  ;; but we don't need to see them.
+  (when name
+(setq name (replace-regexp-in-string "\"" "" name)))
+  ;; If the address is 'foo at bar.com ' then show just
+  ;; 'foo at bar.com'.
+  (when (string= name address)
+(setq name nil))
+
+  (if (not name)
+address
+(concat name " <" address ">")))
+(error address)))

 (defun notmuch-show-insert-headerline (headers date tags depth)
   "Insert a notmuch style headerline based on HEADERS for a
-- 
1.7.5.1



release-candidate/0.6

2011-05-12 Thread Pieter Praet
On Thu, 12 May 2011 09:18:48 -0400, Austin Clements  wrote:
> On Thu, May 12, 2011 at 8:22 AM, Pieter Praet  wrote:
> > The atomicity tests were failing here because I didn't have GDB
> > installed, so I've added it as a prereq.
> 
> Sorry, I've had a patch to address that sitting around, but hadn't
> sent it out (and I only fixed that one test).  I would suggest a
> somewhat gentler approach than "error", though:
> 
> if test_expect_success "prereq: gdb is present" "which gdb"; then
> test_set_prereq GDB
> fi
> 
> (Plus the two test-lib patches I just sent:
> id:1305206080-17461-1-git-send-email-amdragon at mit.edu and
> id:1305206110-17511-1-git-send-email-amdragon at mit.edu).
> 
> "error" has the disadvantage that it doesn't get counted as a failed
> test in the final tally (because, indeed, it's not a failed test) and
> also that it immediately terminates the test script so it's not
> actually using the prereq system (which is fine for the atomicity test
> since all of the test cases depend on GDB, but the pattern I'm
> proposing works for finer-grained prerequisites).  Plus, with the
> above approach, if you don't have a prerequisite, the final tally
> shows one failed test plus some number of skipped tests (and the total
> number of tests never changes), which I would argue is cleaner.

Much obliged for the correction!


Peace


[0] git://github.com/praet/notmuch.git
for-review/test-prereqs-v2
c9a785fc5c48db13

-- 
Pieter


release-candidate/0.6

2011-05-12 Thread Carl Worth
On Fri, 06 May 2011 12:46:34 -0700, Jameson Graef Rollins  wrote:
> Hi, folks.  As some of you already know, I am trying to put together a
> release candidate for a 0.6 release that we can present to cworth for
> approval.

Hi Jameson,

This is really great! I appreciate you collecting useful patches for
review like this. I hope this helps us get things moving once again.

> So far, this release candidate includes a couple of patch series that
> are not currently on cworth's master branch:
> 
> json structure now replicates mime structure
> * dme's json-fully-reflects-MIME-structure improvements

I got stalled on the first commit on this branch:

commit 5a5aae66bf41d3c621c412da711fec9b33f37dcc
Author: David Edmondson 
Date:   Mon May 10 10:25:15 2010 +0100

Improved MIME support.

Signed-off-by: Jameson Rollins 

It's probably the same issue that stalled me when David first sent this
patch series for review over a year ago[!]. I'm guessing I didn't do a
good job of stating the issue then, so I'll try to do a better job here.

This first patch appears to be doing multiple things:

   * It changes how the emacs code inserts MIME parts when constructing
 a mail view.

   * It changes something in the json formatting. (Just extra newlines?)

   * It changes something about the MIME part counting.

   * It appears to be doing new emission of multipart part when doing
 json output, (and for this has some hard-coded printf("]}\n") stuff
 rather than using the formatter).

   * It has new explicit support for an embedded Message as a MIME part,
 (inserting the headers of the enclosed message).

So that's five seemingly independent changes. I'd really like to see
those split up into separate commits as much as possible. Or, excepting
that, there should be some explanation/justification in the commit
message for why some must be combined.

The worst part is that not a single change is actually described in the
commit message. All we have is "Improved MIME support". Improved how?
What changed? Why? What impact does it have? Does it fix bugs? Lay
groundwork for future changes?

What's really changing here and why?

If the patch series were sufficiently-well described in the commit
message, then my review process could be more or less:

  * Read the commit message. Does it describe a desirable change?

  * If so, read the patch content. Does it do what the commit message
describes? Accurately? Without doing unrelated things?

  * If so, accept the patch.

Does anyone want to attempt to fix up this first patch? (It doesn't
necessarily have to be David).


[PATCH] test: "search-by-folder" single-word search terms should be updated correctly when directories are renamed.

2011-05-12 Thread Pieter Praet
On Sun, 13 Mar 2011 00:44:14 +0100, Pieter Praet  wrote:
> On Mon, 17 Jan 2011 14:00:41 +0100, Sebastian Spaeth  SSpaeth.de> wrote:
> > However, when moving files into another folder and rerunning notmuch
> > new, the folder value is never updated, it seems.
> 
> This appears to occur only when searching with a single-word folder
> path.
> 
> ---
>  test/search-by-folder |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/test/search-by-folder b/test/search-by-folder
> index 162158c..63ed57b 100755
> --- a/test/search-by-folder
> +++ b/test/search-by-folder
> @@ -28,13 +28,17 @@ notmuch new
>  output=$(notmuch search folder:bad/news | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test 
> Suite; Bears (inbox unread)"
> 
> -test_begin_subtest "After rename, old path returns nothing"
> +test_begin_subtest "After rename, old multi-word path returns nothing"
>  mv "${MAIL_DIR}/duplicate/bad/news" "${MAIL_DIR}/duplicate/bad/olds"
>  increment_mtime "${MAIL_DIR}/duplicate/bad"
>  notmuch new
>  output=$(notmuch search folder:bad/news | notmuch_search_sanitize)
>  test_expect_equal "$output" ""
> 
> +test_begin_subtest "After rename, old single-word path returns nothing"
> +output=$(notmuch search folder:news | notmuch_search_sanitize)
> +test_expect_equal "$output" ""
> +
>  test_begin_subtest "After rename, new path returns result"
>  output=$(notmuch search folder:bad/olds | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test 
> Suite; Bears (inbox unread)"
> ---

I've pushed a cleaner version of this patch to my github repo, for
inclusion in release-candidate/0.6.


Peace


[0] git://github.com/praet/notmuch.git
for-review/test-folder-search
da50c58545c2fbb77fed


-- 
Pieter


release-candidate/0.6

2011-05-12 Thread Pieter Praet
On Tue, 10 May 2011 09:42:39 -0700, Jameson Graef Rollins  wrote:
> Arg.  One last bit of churn.
> 
> dkg found a bug in the new sanitize_string function that was causing
> segfaults on messages with empty headers.  This is obviously an imprtant
> thing to fix.
> 
> After chatting with some folks on #notmuch, we decided that the debian
> build dependency on libgmime 2.4.24 is a bit too extreme, particularly
> since it will actually prevent the package from being uploaded to
> debian.  I therefore rolled-back the stricter build dependency.
> 
> A new version of release-candidate/0.6 is pushed.

The atomicity tests were failing here because I didn't have GDB
installed, so I've added it as a prereq.

While I was at it, I "fixed" the crypto and emacs tests as well.

> jamie.
> 
> [0] git://finestructure.net/notmuch
> release-candidate/0.6
> aac1936bc9f878bd9ad2e097c5a686e146b73143
Non-text part: application/pgp-signature
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Peace


[0] git://github.com/praet/notmuch.git
for-review/test-prereqs
a845ca793873b6ab637a

-- 
Pieter


release-candidate/0.6

2011-05-12 Thread Austin Clements
Nifty.  I was afraid to go romping through all of the other test
dependencies; I'm glad somebody wasn't.  ]:--8)

It should be noted that these patches depend on
id:1305206110-17511-1-git-send-email-amdragon at mit.edu for correctness
and id:1305206080-17461-1-git-send-email-amdragon at mit.edu for sanity.

Given that these patches aren't specifically 0.6-related and that it's
good practice to send patches to the list, perhaps you could git
send-email these patches as a new thread to the list?

On Thu, May 12, 2011 at 10:09 AM, Pieter Praet  wrote:
> On Thu, 12 May 2011 09:18:48 -0400, Austin Clements  
> wrote:
>> On Thu, May 12, 2011 at 8:22 AM, Pieter Praet  wrote:
>> > The atomicity tests were failing here because I didn't have GDB
>> > installed, so I've added it as a prereq.
>>
>> Sorry, I've had a patch to address that sitting around, but hadn't
>> sent it out (and I only fixed that one test). ?I would suggest a
>> somewhat gentler approach than "error", though:
>>
>> if test_expect_success "prereq: gdb is present" "which gdb"; then
>> ? ? test_set_prereq GDB
>> fi
>>
>> (Plus the two test-lib patches I just sent:
>> id:1305206080-17461-1-git-send-email-amdragon at mit.edu and
>> id:1305206110-17511-1-git-send-email-amdragon at mit.edu).
>>
>> "error" has the disadvantage that it doesn't get counted as a failed
>> test in the final tally (because, indeed, it's not a failed test) and
>> also that it immediately terminates the test script so it's not
>> actually using the prereq system (which is fine for the atomicity test
>> since all of the test cases depend on GDB, but the pattern I'm
>> proposing works for finer-grained prerequisites). ?Plus, with the
>> above approach, if you don't have a prerequisite, the final tally
>> shows one failed test plus some number of skipped tests (and the total
>> number of tests never changes), which I would argue is cleaner.
>
> Much obliged for the correction!
>
>
> Peace
>
>
> [0] git://github.com/praet/notmuch.git
> ? ?for-review/test-prereqs-v2
> ? ?c9a785fc5c48db13
>
> --
> Pieter
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>


release-candidate/0.6

2011-05-12 Thread Austin Clements
On Thu, May 12, 2011 at 8:22 AM, Pieter Praet  wrote:
> The atomicity tests were failing here because I didn't have GDB
> installed, so I've added it as a prereq.

Sorry, I've had a patch to address that sitting around, but hadn't
sent it out (and I only fixed that one test).  I would suggest a
somewhat gentler approach than "error", though:

if test_expect_success "prereq: gdb is present" "which gdb"; then
test_set_prereq GDB
fi

(Plus the two test-lib patches I just sent:
id:1305206080-17461-1-git-send-email-amdragon at mit.edu and
id:1305206110-17511-1-git-send-email-amdragon at mit.edu).

"error" has the disadvantage that it doesn't get counted as a failed
test in the final tally (because, indeed, it's not a failed test) and
also that it immediately terminates the test script so it's not
actually using the prereq system (which is fine for the atomicity test
since all of the test cases depend on GDB, but the pattern I'm
proposing works for finer-grained prerequisites).  Plus, with the
above approach, if you don't have a prerequisite, the final tally
shows one failed test plus some number of skipped tests (and the total
number of tests never changes), which I would argue is cleaner.


[PATCH] test: Report test failures from test_expect_*

2011-05-12 Thread Austin Clements
This makes test_expect_* return non-zero if the test fails, so the
caller can make decisions based on this, such as setting test
prerequisites.
---
 test/test-lib.sh |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 9b56406..955350a 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -575,6 +575,7 @@ test_failure_ () {
echo "$@" | sed -e 's/^//'
if test "$verbose" != "t"; then cat test.output; fi
test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+   return 1
 }

 test_known_broken_ok_ () {
@@ -587,6 +588,7 @@ test_known_broken_failure_ () {
test_broken=$(($test_broken+1))
say_color pass "%-6s" "BROKEN"
echo " $@"
+   return 1
 }

 test_debug () {
-- 
1.7.5.1



[PATCH] test: Fix message when skipping test_expect_equal* tests

2011-05-12 Thread Austin Clements
For these types of tests, the test name is previously recorded in a
variable, not passed to the test function, so pass this variable to
test_skip.
---
 test/test-lib.sh |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index eaf5051..9b56406 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -448,7 +448,7 @@ test_expect_equal ()

output="$1"
expected="$2"
-   if ! test_skip "$@"
+   if ! test_skip "$test_subtest_name"
then
if [ "$output" = "$expected" ]; then
test_ok_ "$test_subtest_name"
@@ -471,7 +471,7 @@ test_expect_equal_file ()

output="$1"
expected="$2"
-   if ! test_skip "$@"
+   if ! test_skip "$test_subtest_name"
then
if diff -q "$expected" "$output" >/dev/null ; then
test_ok_ "$test_subtest_name"
@@ -494,7 +494,7 @@ test_expect_equal_failure ()

output="$1"
expected="$2"
-   if ! test_skip "$@"
+   if ! test_skip "$test_subtest_name"
then
if [ "$output" = "$expected" ]; then
test_known_broken_ok_ "$test_subtest_name"
-- 
1.7.5.1



[PATCH] test: "search-by-folder" single-word search terms should be updated correctly when directories are renamed.

2011-05-12 Thread Jameson Graef Rollins
On Thu, 12 May 2011 15:22:25 +0200, Pieter Praet  wrote:
> I've pushed a cleaner version of this patch to my github repo, for
> inclusion in release-candidate/0.6.

Hey, Pieter.  I realize we've been passing around git urls a lot
recently, particularly in regards to the release-candidate/0.6 branch,
but I think we should try to retain the habit of sending all patches to
the list if we can.

In general, I think we should try to only use git branches when:

- compiling existing patch series for review.
- new patch series that are so long that sending them to the list would
  be cumbersome.

I've been bad about this myself, since I was pushing patches to the
r-c/0.6 branch, but I think in general we should try to avoid that.
It's nice to have all patches on the list, so everyone can easily review
them and publicly comment on them.

Thanks!

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/20110512/e27f0f6c/attachment.pgp>


address completion when composing

2011-05-12 Thread Florian Friesdorf
On Wed, 11 May 2011 10:18:43 -0700, Jameson Graef Rollins  wrote:
> On Wed, 11 May 2011 14:31:36 +0200, Florian Friesdorf  
> wrote:
> > In addition, about a year ago, Sebastian indicated, that notmuch itself
> > could offer returning email addresses [3]. Are there arguments against
> > or just nobody found the time to do it?
> 
> As with pretty much everything in notmuch, manpower time is probably the
> issue.  I think that if you wanted to get this working that would be
> great.

ok, if there is anybody eager to do this - please go on! otherwise, I
would give it a try, but considering my current workload this might take
a bit. But, I promise I will not start using any other address book
solution to increase the pressure.

It would be great if people more knowledgeable about the notmuch code
base could provide some pointers and outline the solution.

What I heard here so far:
1. store to/cc/bcc in xapian
2. provide a mechanism to query those fields from the cmdline

Questions:

- Do we want a concept of
  one-person-several-emails(-for-different-purposes)?

- Do we want to filter only on the email/name data itself or also things
  like:

  * give me all email addresses of people who sent a mail between 1st
and 5th May

  * ... who wrote mails mentioning xyz

  * ... who wrote mails tagged X

  * ... who wrote to foo at bar.com

- Do we want to tag persons/email addresses to create groups. 


A list of "you should read this"-links would be great. 

-- 
Florian Friesdorf 
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo at chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC
-- 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/20110512/fce2d329/attachment.pgp>


storing From and Subject in xapian

2011-05-12 Thread Istvan Marko
Stewart Smith  writes:

> Would it be possible to progressively fill the DB with the new data?
>
> i.e.
>
> if Subject/From not in db for message
>add Subject/From for this message to DB.

I started looking into this but then realized that notmuch search opens
the database in read-only mode so it cannot make updates. It might be
desirable to keep it that way for safety and locking reasons.

-- 
Istvan


address completion when composing

2011-05-12 Thread Jameson Graef Rollins
On Thu, 12 May 2011 02:04:41 +0200, Florian Friesdorf  
wrote:
> 1. store to/cc/bcc in xapian

Hey, Florian.  I think all of these fields are actually already stored
in the database.

> 2. provide a mechanism to query those fields from the cmdline

I think this is the idea.  You would also want to match against subsets
of the fields, and output all matching fields.

I think just starting with this, without worrying about any other
options, would be a great way to start, and useful in and of itself.

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/20110512/4b0f5419/attachment.pgp>


Re: address completion when composing

2011-05-12 Thread Jameson Graef Rollins
On Thu, 12 May 2011 02:04:41 +0200, Florian Friesdorf f...@chaoflow.net wrote:
 1. store to/cc/bcc in xapian

Hey, Florian.  I think all of these fields are actually already stored
in the database.

 2. provide a mechanism to query those fields from the cmdline

I think this is the idea.  You would also want to match against subsets
of the fields, and output all matching fields.

I think just starting with this, without worrying about any other
options, would be a great way to start, and useful in and of itself.

jamie.


pgp64KfGoDGor.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: storing From and Subject in xapian

2011-05-12 Thread Istvan Marko
Stewart Smith stew...@flamingspork.com writes:

 Would it be possible to progressively fill the DB with the new data?

 i.e.

 if Subject/From not in db for message
add Subject/From for this message to DB.

I started looking into this but then realized that notmuch search opens
the database in read-only mode so it cannot make updates. It might be
desirable to keep it that way for safety and locking reasons.

-- 
Istvan
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: release-candidate/0.6

2011-05-12 Thread Pieter Praet
On Tue, 10 May 2011 09:42:39 -0700, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 Arg.  One last bit of churn.
 
 dkg found a bug in the new sanitize_string function that was causing
 segfaults on messages with empty headers.  This is obviously an imprtant
 thing to fix.
 
 After chatting with some folks on #notmuch, we decided that the debian
 build dependency on libgmime 2.4.24 is a bit too extreme, particularly
 since it will actually prevent the package from being uploaded to
 debian.  I therefore rolled-back the stricter build dependency.
 
 A new version of release-candidate/0.6 is pushed.

The atomicity tests were failing here because I didn't have GDB
installed, so I've added it as a prereq.

While I was at it, I fixed the crypto and emacs tests as well.

 jamie.
 
 [0] git://finestructure.net/notmuch
 release-candidate/0.6
 aac1936bc9f878bd9ad2e097c5a686e146b73143
Non-text part: application/pgp-signature
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch

Peace


[0] git://github.com/praet/notmuch.git
for-review/test-prereqs
a845ca793873b6ab637a

-- 
Pieter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: Fix message when skipping test_expect_equal* tests

2011-05-12 Thread Austin Clements
For these types of tests, the test name is previously recorded in a
variable, not passed to the test function, so pass this variable to
test_skip.
---
 test/test-lib.sh |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index eaf5051..9b56406 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -448,7 +448,7 @@ test_expect_equal ()
 
output=$1
expected=$2
-   if ! test_skip $@
+   if ! test_skip $test_subtest_name
then
if [ $output = $expected ]; then
test_ok_ $test_subtest_name
@@ -471,7 +471,7 @@ test_expect_equal_file ()
 
output=$1
expected=$2
-   if ! test_skip $@
+   if ! test_skip $test_subtest_name
then
if diff -q $expected $output /dev/null ; then
test_ok_ $test_subtest_name
@@ -494,7 +494,7 @@ test_expect_equal_failure ()
 
output=$1
expected=$2
-   if ! test_skip $@
+   if ! test_skip $test_subtest_name
then
if [ $output = $expected ]; then
test_known_broken_ok_ $test_subtest_name
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH] test: Report test failures from test_expect_*

2011-05-12 Thread Austin Clements
This makes test_expect_* return non-zero if the test fails, so the
caller can make decisions based on this, such as setting test
prerequisites.
---
 test/test-lib.sh |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 9b56406..955350a 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -575,6 +575,7 @@ test_failure_ () {
echo $@ | sed -e 's/^//'
if test $verbose != t; then cat test.output; fi
test $immediate =  || { GIT_EXIT_OK=t; exit 1; }
+   return 1
 }
 
 test_known_broken_ok_ () {
@@ -587,6 +588,7 @@ test_known_broken_failure_ () {
test_broken=$(($test_broken+1))
say_color pass %-6s BROKEN
echo  $@
+   return 1
 }
 
 test_debug () {
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: release-candidate/0.6

2011-05-12 Thread Austin Clements
On Thu, May 12, 2011 at 8:22 AM, Pieter Praet pie...@praet.org wrote:
 The atomicity tests were failing here because I didn't have GDB
 installed, so I've added it as a prereq.

Sorry, I've had a patch to address that sitting around, but hadn't
sent it out (and I only fixed that one test).  I would suggest a
somewhat gentler approach than error, though:

if test_expect_success prereq: gdb is present which gdb; then
test_set_prereq GDB
fi

(Plus the two test-lib patches I just sent:
id:1305206080-17461-1-git-send-email-amdra...@mit.edu and
id:1305206110-17511-1-git-send-email-amdra...@mit.edu).

error has the disadvantage that it doesn't get counted as a failed
test in the final tally (because, indeed, it's not a failed test) and
also that it immediately terminates the test script so it's not
actually using the prereq system (which is fine for the atomicity test
since all of the test cases depend on GDB, but the pattern I'm
proposing works for finer-grained prerequisites).  Plus, with the
above approach, if you don't have a prerequisite, the final tally
shows one failed test plus some number of skipped tests (and the total
number of tests never changes), which I would argue is cleaner.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: search-by-folder single-word search terms should be updated correctly when directories are renamed.

2011-05-12 Thread Pieter Praet
On Sun, 13 Mar 2011 00:44:14 +0100, Pieter Praet pie...@praet.org wrote:
 On Mon, 17 Jan 2011 14:00:41 +0100, Sebastian Spaeth sebast...@sspaeth.de 
 wrote:
  However, when moving files into another folder and rerunning notmuch
  new, the folder value is never updated, it seems.
 
 This appears to occur only when searching with a single-word folder
 path.
 
 ---
  test/search-by-folder |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)
 
 diff --git a/test/search-by-folder b/test/search-by-folder
 index 162158c..63ed57b 100755
 --- a/test/search-by-folder
 +++ b/test/search-by-folder
 @@ -28,13 +28,17 @@ notmuch new
  output=$(notmuch search folder:bad/news | notmuch_search_sanitize)
  test_expect_equal $output thread:XXX   2001-01-05 [1/1] Notmuch Test 
 Suite; Bears (inbox unread)
 
 -test_begin_subtest After rename, old path returns nothing
 +test_begin_subtest After rename, old multi-word path returns nothing
  mv ${MAIL_DIR}/duplicate/bad/news ${MAIL_DIR}/duplicate/bad/olds
  increment_mtime ${MAIL_DIR}/duplicate/bad
  notmuch new
  output=$(notmuch search folder:bad/news | notmuch_search_sanitize)
  test_expect_equal $output 
 
 +test_begin_subtest After rename, old single-word path returns nothing
 +output=$(notmuch search folder:news | notmuch_search_sanitize)
 +test_expect_equal $output 
 +
  test_begin_subtest After rename, new path returns result
  output=$(notmuch search folder:bad/olds | notmuch_search_sanitize)
  test_expect_equal $output thread:XXX   2001-01-05 [1/1] Notmuch Test 
 Suite; Bears (inbox unread)
 ---

I've pushed a cleaner version of this patch to my github repo, for
inclusion in release-candidate/0.6.


Peace


[0] git://github.com/praet/notmuch.git
for-review/test-folder-search
da50c58545c2fbb77fed


-- 
Pieter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 1/2] Make `notmuch-show-clean-address' parsing-error-proof.

2011-05-12 Thread Dmitry Kurochkin
Mail-header-parse-address may fail for an invalid address.
Before the change, this would result in empty notmuch-show buffer
with an error message like: Scan error: Unbalanced parentheses.
The patch wraps the function in condition-case and returns
unchanged address in case of error.
---
 emacs/notmuch-show.el |   35 +++
 1 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 23291ce..bd348e1 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -224,22 +224,25 @@ same as that of the previous message.
 )))
 
 (defun notmuch-show-clean-address (address)
-  Clean a single email address for display.
-  (let* ((parsed (mail-header-parse-address address))
-(address (car parsed))
-(name (cdr parsed)))
-;; Remove double quotes. They might be required during transport,
-;; but we don't need to see them.
-(when name
-  (setq name (replace-regexp-in-string \  name)))
-;; If the address is 'f...@bar.com f...@bar.com' then show just
-;; 'f...@bar.com'.
-(when (string= name address)
-  (setq name nil))
-
-(if (not name)
-   address
-  (concat name   address 
+  Try to clean a single email ADDRESS for display.  Return
+unchanged ADDRESS if parsing fails.
+  (condition-case nil
+(let* ((parsed (mail-header-parse-address address))
+  (address (car parsed))
+  (name (cdr parsed)))
+  ;; Remove double quotes. They might be required during transport,
+  ;; but we don't need to see them.
+  (when name
+(setq name (replace-regexp-in-string \  name)))
+  ;; If the address is 'f...@bar.com f...@bar.com' then show just
+  ;; 'f...@bar.com'.
+  (when (string= name address)
+(setq name nil))
+
+  (if (not name)
+address
+(concat name   address )))
+(error address)))
 
 (defun notmuch-show-insert-headerline (headers date tags depth)
   Insert a notmuch style headerline based on HEADERS for a
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH 2/2] test: add notmuch-show for message with invalid From test

2011-05-12 Thread Dmitry Kurochkin
---
 test/emacs |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 2e85b0c..f2983a7 100755
--- a/test/emacs
+++ b/test/emacs
@@ -37,8 +37,21 @@ output=$(test_emacs (notmuch-show 
\$maildir_storage_thread\) (princ (buffer-s
 expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
 test_expect_equal $output $expected
 
+test_begin_subtest notmuch-show for message with invalid From
+add_message [subject]=\message-with-invalid-from\ [from]=Invalid 
\\\ From\\\ test_su...@notmuchmail.org\
+thread=$(notmuch search --output=threads subject:message-with-invalid-from)
+output=$(test_emacs (notmuch-show \$thread\) (princ (buffer-string)))
+test_expect_equal $output \
+'Invalid  From test_su...@notmuchmail.org (2001-01-05) (inbox)
+Subject: message-with-invalid-from
+To: Notmuch Test Suite test_su...@notmuchmail.org
+Date: Tue, 05 Jan 2001 15:43:57 -
+
+This is just a test message (#1)'
+
 test_begin_subtest Navigation of notmuch-search to thread view
 output=$(test_emacs '(notmuch-search tag:inbox) (notmuch-test-wait) 
(goto-char (point-min)) (re-search-forward Working with Maildir) 
(notmuch-search-show-thread) (notmuch-test-wait) (princ (buffer-string))')
+expected=$(cat $EXPECTED/notmuch-show-thread-maildir-storage)
 test_expect_equal $output $expected
 
 test_begin_subtest Add tag from search view
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: release-candidate/0.6

2011-05-12 Thread Pieter Praet
On Thu, 12 May 2011 09:18:48 -0400, Austin Clements amdra...@mit.edu wrote:
 On Thu, May 12, 2011 at 8:22 AM, Pieter Praet pie...@praet.org wrote:
  The atomicity tests were failing here because I didn't have GDB
  installed, so I've added it as a prereq.
 
 Sorry, I've had a patch to address that sitting around, but hadn't
 sent it out (and I only fixed that one test).  I would suggest a
 somewhat gentler approach than error, though:
 
 if test_expect_success prereq: gdb is present which gdb; then
 test_set_prereq GDB
 fi
 
 (Plus the two test-lib patches I just sent:
 id:1305206080-17461-1-git-send-email-amdra...@mit.edu and
 id:1305206110-17511-1-git-send-email-amdra...@mit.edu).
 
 error has the disadvantage that it doesn't get counted as a failed
 test in the final tally (because, indeed, it's not a failed test) and
 also that it immediately terminates the test script so it's not
 actually using the prereq system (which is fine for the atomicity test
 since all of the test cases depend on GDB, but the pattern I'm
 proposing works for finer-grained prerequisites).  Plus, with the
 above approach, if you don't have a prerequisite, the final tally
 shows one failed test plus some number of skipped tests (and the total
 number of tests never changes), which I would argue is cleaner.

Much obliged for the correction!


Peace


[0] git://github.com/praet/notmuch.git
for-review/test-prereqs-v2
c9a785fc5c48db13

-- 
Pieter
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: [PATCH] test: search-by-folder single-word search terms should be updated correctly when directories are renamed.

2011-05-12 Thread Jameson Graef Rollins
On Thu, 12 May 2011 15:22:25 +0200, Pieter Praet pie...@praet.org wrote:
 I've pushed a cleaner version of this patch to my github repo, for
 inclusion in release-candidate/0.6.

Hey, Pieter.  I realize we've been passing around git urls a lot
recently, particularly in regards to the release-candidate/0.6 branch,
but I think we should try to retain the habit of sending all patches to
the list if we can.

In general, I think we should try to only use git branches when:

- compiling existing patch series for review.
- new patch series that are so long that sending them to the list would
  be cumbersome.

I've been bad about this myself, since I was pushing patches to the
r-c/0.6 branch, but I think in general we should try to avoid that.
It's nice to have all patches on the list, so everyone can easily review
them and publicly comment on them.

Thanks!

jamie.


pgpHqCSfUMIJw.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


Re: release-candidate/0.6

2011-05-12 Thread Austin Clements
Nifty.  I was afraid to go romping through all of the other test
dependencies; I'm glad somebody wasn't.  ]:--8)

It should be noted that these patches depend on
id:1305206110-17511-1-git-send-email-amdra...@mit.edu for correctness
and id:1305206080-17461-1-git-send-email-amdra...@mit.edu for sanity.

Given that these patches aren't specifically 0.6-related and that it's
good practice to send patches to the list, perhaps you could git
send-email these patches as a new thread to the list?

On Thu, May 12, 2011 at 10:09 AM, Pieter Praet pie...@praet.org wrote:
 On Thu, 12 May 2011 09:18:48 -0400, Austin Clements amdra...@mit.edu wrote:
 On Thu, May 12, 2011 at 8:22 AM, Pieter Praet pie...@praet.org wrote:
  The atomicity tests were failing here because I didn't have GDB
  installed, so I've added it as a prereq.

 Sorry, I've had a patch to address that sitting around, but hadn't
 sent it out (and I only fixed that one test).  I would suggest a
 somewhat gentler approach than error, though:

 if test_expect_success prereq: gdb is present which gdb; then
     test_set_prereq GDB
 fi

 (Plus the two test-lib patches I just sent:
 id:1305206080-17461-1-git-send-email-amdra...@mit.edu and
 id:1305206110-17511-1-git-send-email-amdra...@mit.edu).

 error has the disadvantage that it doesn't get counted as a failed
 test in the final tally (because, indeed, it's not a failed test) and
 also that it immediately terminates the test script so it's not
 actually using the prereq system (which is fine for the atomicity test
 since all of the test cases depend on GDB, but the pattern I'm
 proposing works for finer-grained prerequisites).  Plus, with the
 above approach, if you don't have a prerequisite, the final tally
 shows one failed test plus some number of skipped tests (and the total
 number of tests never changes), which I would argue is cleaner.

 Much obliged for the correction!


 Peace


 [0] git://github.com/praet/notmuch.git
    for-review/test-prereqs-v2
    c9a785fc5c48db13

 --
 Pieter
 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch