Extend test coverage wrt tagging

Extend test coverage wrt tagging

[Pieter Praet]

Some tagging-related tests and a (soon to be deprecated) bugfix,
which should be merged in *before* the (upcoming) race condition
patch series [1], to prevent surprises of the non-enjoyable kind.


Peace

-- 
Pieter


[1] id:"CAH-f9WticM4EN8F1_ik_-mcBcBtrXwSpO+Drbtp7=UN7McECrg@mail.gmail.com"

[PATCH 1/6] test: emacs: rename subtest "Reply within emacs" to "Compose reply in emacs"

[Pieter Praet]

The test populates a reply buffer but doesn't really submit it,
so its title is misleading.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index 53f455a..ac3a246 100755
--- a/test/emacs
+++ b/test/emacs
@@ -239,7 +239,7 @@ Subject:
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "Reply within emacs"
+test_begin_subtest "Compose reply in emacs"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
 	    (notmuch-test-wait)
 	    (notmuch-search-reply-to-thread)
-- 
1.7.5.4

[PATCH 2/6] test: emacs: add test for sending a reply from within Emacs

[Pieter Praet]

As pointed out in the previous commit, the test for replying from
within Emacs didn't actually submit the reply. This one does.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index ac3a246..f277653 100755
--- a/test/emacs
+++ b/test/emacs
@@ -257,6 +257,49 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Send reply from within Emacs"
+$TEST_DIRECTORY/smtp-dummy sent_message &
+smtp_dummy_pid=$!
+test_emacs \
+'(let ((message-send-mail-function '\''message-smtpmail-send-it)
+       (smtpmail-smtp-server "localhost")
+       (smtpmail-smtp-service "25025"))
+  (notmuch-search "subject:\"testing message sent via SMTP\"")
+  (notmuch-test-wait)
+  (notmuch-search-reply-to-thread)
+  (message-goto-to)
+  (message-goto-body)
+  (end-of-buffer)
+  (newline)
+  (insert "Reply to a message via Emacs with fake SMTP")
+  (message-send-and-exit))' >/dev/null 2>&1
+wait ${smtp_dummy_pid}
+notmuch new >/dev/null
+sed \
+    -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
+    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' \
+    -e s',^In-Reply-To: <.*>$,In-Reply-To: <XXX>,' \
+    -e s',^References: <.*>$,References: <XXX>,' \
+    -e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -0000,' < sent_message >OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: user@example.com
+Subject: Re: Testing message sent via SMTP
+In-Reply-To: <XXX>
+References: <XXX>
+User-Agent: Notmuch/XXX Emacs/XXX
+Date: Fri, 29 Mar 1974 10:05:00 -0000
+Message-ID: <XXX>
+MIME-Version: 1.0
+Content-Type: text/plain; charset=us-ascii
+
+On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+> This is a test that messages are sent via SMTP
+
+Reply to a message via Emacs with fake SMTP
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment1.gz\""))
-- 
1.7.5.4

[PATCH 3/6] test: emacs: add test to verify that the message being replied to is tagged as such

[Pieter Praet]

When a message is replied to, it should be tagged `replied'.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index f277653..6479c4e 100755
--- a/test/emacs
+++ b/test/emacs
@@ -300,6 +300,10 @@ Reply to a message via Emacs with fake SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "Verify that 'replied' tag is added to reply's parent message."
+output=$(notmuch search 'tag:replied' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2000-01-01 [1/2] Notmuch Test Suite; Testing message sent via SMTP (inbox replied)"
+
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment1.gz\""))
-- 
1.7.5.4

[PATCH 4/6] test: emacs: add test for `notmuch-search-operate-all'

[Pieter Praet]

`notmuch-search-operate-all' (bound to "*") adds and removes tags
to/from all messages which match the query used to populate the
current search buffer.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs-search-operate-all |   29 +++++++++++++++++++++++++++++
 test/notmuch-test             |    1 +
 2 files changed, 30 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-search-operate-all

diff --git a/test/emacs-search-operate-all b/test/emacs-search-operate-all
new file mode 100755
index 0000000..2bd50ba
--- /dev/null
+++ b/test/emacs-search-operate-all
@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+
+test_description="emacs interface"
+. test-lib.sh
+
+EXPECTED=$TEST_DIRECTORY/emacs.expected-output
+
+add_email_corpus
+
+test_begin_subtest "Add/remove tags to/from all matching messages."
+test_emacs '(notmuch-search "tag:inbox AND tags")
+	    (notmuch-test-wait)
+	    (notmuch-search-operate-all "+matching -inbox")
+	    (notmuch-search "tag:matching AND NOT tag:inbox")
+	    (notmuch-test-wait)
+	    (test-output)'
+cat <<EOF >EXPECTED
+  2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
+  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
+  2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
+  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
+  2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
+  2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
+  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
+End of search results.
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 79e6267..bafbcb1 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -38,6 +38,7 @@ TESTS="
   encoding
   emacs
   emacs-large-search-buffer
+  emacs-search-operate-all
   maildir-sync
   crypto
   symbol-hiding
-- 
1.7.5.4

[PATCH 5/6] test: emacs: `notmuch-search-operate-all' should operate on threads, not messages

[Pieter Praet]

In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
`notmuch-search-operate-all' ("*") should operate on matching threads
instead of matching messages.

This test exposes an existing bug, so is currently failing.
It should be fixed in the next commit.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs-search-operate-all |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/emacs-search-operate-all b/test/emacs-search-operate-all
index 2bd50ba..48326c8 100755
--- a/test/emacs-search-operate-all
+++ b/test/emacs-search-operate-all
@@ -7,7 +7,7 @@ EXPECTED=$TEST_DIRECTORY/emacs.expected-output
 
 add_email_corpus
 
-test_begin_subtest "Add/remove tags to/from all matching messages."
+test_begin_subtest "Add/remove tags to/from all matching threads."
 test_emacs '(notmuch-search "tag:inbox AND tags")
 	    (notmuch-test-wait)
 	    (notmuch-search-operate-all "+matching -inbox")
@@ -15,13 +15,13 @@ test_emacs '(notmuch-search "tag:inbox AND tags")
 	    (notmuch-test-wait)
 	    (test-output)'
 cat <<EOF >EXPECTED
+  2009-11-18 [2/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (matching unread)
   2009-11-18 [3/3]   Adrian Perez de Castro, Keith Packard, Carl Worth  [notmuch] Introducing myself (matching signed unread)
-  2009-11-18 [1/3]   Carl Worth, Israel Herraiz, Keith Packard       [notmuch] New to the list (inbox matching unread)
+  2009-11-18 [3/3]   Israel Herraiz, Keith Packard, Carl Worth   [notmuch] New to the list (matching unread)
   2009-11-18 [2/2]   Keith Packard, Carl Worth    [notmuch] [PATCH] Make notmuch-show 'X' (and 'x') commands remove inbox (and unread) tags (matching unread)
-  2009-11-18 [1/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (inbox matching unread)
+  2009-11-18 [2/2]   Keith Packard, Alexander Botero-Lowry    [notmuch] [PATCH] Create a default notmuch-show-hook that highlights URLs and uses word-wrap (matching unread)
   2009-11-18 [1/1]   Jan Janak            [notmuch] [PATCH] notmuch new: Support for conversion of spool subdirectories into tags (matching unread)
   2009-11-18 [1/1]   Stewart Smith        [notmuch] [PATCH] Fix linking with gcc to use g++ to link in C++ libs. (matching unread)
-  2009-11-17 [1/2]   Ingmar Vanhassel, Carl Worth  [notmuch] [PATCH] Typsos (inbox matching unread)
 End of search results.
 EOF
 test_expect_equal_file OUTPUT EXPECTED
-- 
1.7.5.4

[PATCH 6/6] emacs: make `notmuch-search-operate-all' operate on threads, not messages

[Pieter Praet]

In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
`notmuch-search-operate-all' ("*") should operate on matching threads
instead of matching messages.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 emacs/notmuch.el |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f11ec24..07899b7 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -834,9 +834,9 @@ non-authors is found, assume that all of the authors match."
       (delete-process proc))))
 
 (defun notmuch-search-operate-all (action)
-  "Add/remove tags from all matching messages.
+  "Add/remove tags from all matching threads.
 
-This command adds or removes tags from all messages matching the
+This command adds or removes tags from all threads matching the
 current search terms. When called interactively, this command
 will prompt for tags to be added or removed. Tags prefixed with
 '+' will be added and tags prefixed with '-' will be removed.
@@ -845,7 +845,10 @@ Each character of the tag name may consist of alphanumeric
 characters as well as `_.+-'.
 "
   (interactive "sOperation (+add -drop): notmuch tag ")
-  (let ((action-split (split-string action " +")))
+  (let ((action-split (split-string action " +"))
+        (threads (mapconcat 'identity
+                           (notmuch-search-find-thread-id-region (point-min) (- (point-max) 2))
+                           " or ")))
     ;; Perform some validation
     (let ((words action-split))
       (when (null words) (error "No operation given"))
@@ -853,7 +856,7 @@ characters as well as `_.+-'.
 	(unless (string-match-p "^[-+][-+_.[:word:]]+$" (car words))
 	  (error "Action must be of the form `+thistag -that_tag'"))
 	(setq words (cdr words))))
-    (apply 'notmuch-tag notmuch-search-query-string action-split)))
+    (apply 'notmuch-tag threads action-split)))
 
 (defun notmuch-search-buffer-title (query)
   "Returns the title for a buffer with notmuch search results."
-- 
1.7.5.4

Re: [PATCH 6/6] emacs: make `notmuch-search-operate-all' operate on threads, not messages

[David Bremner]

On Sun, 10 Jul 2011 17:55:35 +0200, Pieter Praet <pieter@praet.org> wrote:
> In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
> `notmuch-search-operate-all' ("*") should operate on matching threads
> instead of matching messages.
> 

As far as I can tell, the follow-up series for the race condition kindof
got stalled.  Am I right in thinking this bug fix should still be
applied? I didn't see any review/feedback on the list.

d

Re: [PATCH 6/6] emacs: make `notmuch-search-operate-all' operate on threads, not messages

[Austin Clements]

Quoth David Bremner on Nov 12 at 11:13 am:
> On Sun, 10 Jul 2011 17:55:35 +0200, Pieter Praet <pieter@praet.org> wrote:
> > In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
> > `notmuch-search-operate-all' ("*") should operate on matching threads
> > instead of matching messages.
> > 
> 
> As far as I can tell, the follow-up series for the race condition kindof
> got stalled.  Am I right in thinking this bug fix should still be
> applied? I didn't see any review/feedback on the list.

We concluded that fixing the tagging race correctly was actually a lot
of work, which should be done but hasn't yet.  We have to add message
IDs or docids to the search results, which is difficult to do with the
current text format, so rather than further entrenching ourselves, we
should first we should migrate Emacs to using the JSON-based search
output.

However, this series doesn't actually have much to do with the race.
I think the question here is whether notmuch-search-operate-all should
affect only matched messages or entire threads.  It seems to me it
should affect all threads, since that's what you're seeing visually,
but other people may disagree.

The test patches seem reasonable, though they could use a little
review before being pushed.

Re: [PATCH 6/6] emacs: make `notmuch-search-operate-all' operate on threads, not messages

[Pieter Praet]

On Sat, 12 Nov 2011 11:35:02 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth David Bremner on Nov 12 at 11:13 am:
> > On Sun, 10 Jul 2011 17:55:35 +0200, Pieter Praet <pieter@praet.org> wrote:
> > > In order to be consistent with `notmuch-search-{add,remove}-tag' ("+"/"-"),
> > > `notmuch-search-operate-all' ("*") should operate on matching threads
> > > instead of matching messages.
> > > 
> > 
> > As far as I can tell, the follow-up series for the race condition kindof
> > got stalled.  Am I right in thinking this bug fix should still be
> > applied? I didn't see any review/feedback on the list.
> 
> We concluded that fixing the tagging race correctly was actually a lot
> of work, which should be done but hasn't yet.  We have to add message
> IDs or docids to the search results, which is difficult to do with the
> current text format, so rather than further entrenching ourselves, we
> should first we should migrate Emacs to using the JSON-based search
> output.
> 

Yeah, sorry I haven't replied there yet.  Still haven't found a
sufficiently uninterrupted stretch of time to give the *massive*
amount of work you did the attention it deserves.

> However, this series doesn't actually have much to do with the race.

Correct.  Only patch #4 is more or less relevant to fixing the
`notmuch-search-operate-all' race condition (safety net for when
I make stupid mistakes).

Patches #1-3 should have been in a separate thread (or as updates in
their original thread [1]), but since #1 and #2 are mainly there to
support #3 and #3 is tagging-related, I though it wouldn't hurt to
include them.

Patches #5-6 are a matter of opinion:

> I think the question here is whether notmuch-search-operate-all should
> affect only matched messages or entire threads.  It seems to me it
> should affect all threads, since that's what you're seeing visually,
> but other people may disagree.
> 

Same here.  I don't use it that often, but if its name includes
"operate-all", it should do just that, or the function should be
renamed.

> The test patches seem reasonable, though they could use a little
> review before being pushed.

I'd really appreciate it.

AFAIC, increasing test coverage should be a top priority.


Peace

-- 
Pieter

[1] id:"1305275652-22956-1-git-send-email-pieter@praet.org"

(no subject)

[Pieter Praet]

- Rebased to current master (be851ad3).
- Addressed Dmitry's comments re test for `notmuch-search-operate-all'
  (which is now `notmuch-search-tag-all') [1,2,3].
- Dropped patches 5 and 6 [5,6].  Might be dealt with later.


Peace

[1] id:"87y5spober.fsf@gmail.com"
[2] id:"87hazamywi.fsf@gmail.com"
[3] id:"8762fqm8zv.fsf@gmail.com"
[4] nil :)
[5] id:"1310313335-4159-6-git-send-email-pieter@praet.org"
[6] id:"1310313335-4159-7-git-send-email-pieter@praet.org"

[PATCH v2 1/4] test: emacs: rename subtest "Reply within emacs"

[Pieter Praet]

Rename subtest "Reply within emacs" to "notmuch-search: replying to a
thread (composing)" as its title is misleading: it populates a reply
buffer but doesn't submit it.
---
 test/emacs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/emacs b/test/emacs
index b74cfa9..308d749 100755
--- a/test/emacs
+++ b/test/emacs
@@ -255,7 +255,7 @@ Subject:
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_begin_subtest "Reply within emacs"
+test_begin_subtest "notmuch-search: replying to a thread (composing)"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
 	    (notmuch-test-wait)
 	    (notmuch-search-reply-to-thread)
-- 
1.7.8.1

[PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"

[Pieter Praet]

As pointed out in the previous commit, the test for replying from
within Emacs didn't actually submit the reply. This one does.
---
 test/emacs |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 308d749..0f4f42b 100755
--- a/test/emacs
+++ b/test/emacs
@@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "notmuch-search: replying to a thread (sending)"
+$TEST_DIRECTORY/smtp-dummy sent_message &
+smtp_dummy_pid=$!
+test_emacs \
+'(let ((message-send-mail-function '\''message-smtpmail-send-it)
+       (smtpmail-smtp-server "localhost")
+       (smtpmail-smtp-service "25025"))
+  (notmuch-search "subject:\"testing message sent via SMTP\"")
+  (notmuch-test-wait)
+  (notmuch-search-reply-to-thread)
+  (end-of-buffer)
+  (newline)
+  (insert "Reply to a message via Emacs with fake SMTP")
+  (message-send-and-exit))' >/dev/null 2>&1
+wait ${smtp_dummy_pid}
+notmuch new >/dev/null
+sed \
+    -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
+    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' \
+    -e s',^In-Reply-To: <.*>$,In-Reply-To: <XXX>,' \
+    -e s',^References: <.*>$,References: <XXX>,' \
+    -e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -0000,' < sent_message >OUTPUT
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+To: user@example.com
+Subject: Re: Testing message sent via SMTP
+In-Reply-To: <XXX>
+References: <XXX>
+User-Agent: Notmuch/XXX Emacs/XXX
+Date: Fri, 29 Mar 1974 10:05:00 -0000
+Message-ID: <XXX>
+MIME-Version: 1.0
+Content-Type: text/plain; charset=us-ascii
+
+On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
+> This is a test that messages are sent via SMTP
+
+Reply to a message via Emacs with fake SMTP
+EOF
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Quote MML tags in reply"
 message_id='test-emacs-mml-quoting@message.id'
 add_message [id]="$message_id" \
-- 
1.7.8.1

[PATCH v2 3/4] test: emacs: new test "notmuch-search: when reply is sent, parent message should be tagged 'replied'"

[Pieter Praet]

When a message is replied to, it should be tagged `replied'.
---
 test/emacs |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 0f4f42b..b0fb760 100755
--- a/test/emacs
+++ b/test/emacs
@@ -314,6 +314,11 @@ Reply to a message via Emacs with fake SMTP
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest "notmuch-search: when reply is sent, the parent message should be tagged 'replied'"
+# depends on subtest "notmuch-search: replying to a thread (sending)"
+output=$(notmuch search 'tag:replied' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2000-01-01 [1/2] Notmuch Test Suite; Testing message sent via SMTP (inbox replied)"
+
 test_begin_subtest "Quote MML tags in reply"
 message_id='test-emacs-mml-quoting@message.id'
 add_message [id]="$message_id" \
-- 
1.7.8.1

[PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Pieter Praet]

`notmuch-search-tag-all' (bound to "*") adds and removes tags
to/from all messages which match the query used to populate the
current search buffer.
---
 test/emacs |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index b0fb760..1db8540 100755
--- a/test/emacs
+++ b/test/emacs
@@ -124,6 +124,38 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
+test_begin_subtest "notmuch-search: change tags of all matching messages"
+old_tag="inbox"
+new_tag="xobni"
+filter="AND from:cworth"
+# Get initial tag counts and prevent false positives/negatives
+old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
+test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
+test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
+# Change tags of all matching messages and get tag counts
+test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
+old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
+# Revert tag changes and get tag counts
+test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
+old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
+# ... and verify the results
+output="
+before:   old:${old_tag_count_1} new:${new_tag_count_1}
+after:    old:${old_tag_count_2} new:${new_tag_count_2}
+restored: old:${old_tag_count_3} new:${new_tag_count_3}"
+expected="
+before:   old:${old_tag_count_1} new:0
+after:    old:0 new:${old_tag_count_1}
+restored: old:${old_tag_count_1} new:0"
+test_expect_equal "$output" "$expected"
+
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"")
-- 
1.7.8.1

Re: [PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"

[Tomi Ollila]

On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet <pieter@praet.org> wrote:
> As pointed out in the previous commit, the test for replying from
> within Emacs didn't actually submit the reply. This one does.
> ---
>  test/emacs |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index 308d749..0f4f42b 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> +$TEST_DIRECTORY/smtp-dummy sent_message &
> +smtp_dummy_pid=$!
> +test_emacs \
> +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> +       (smtpmail-smtp-server "localhost")
> +       (smtpmail-smtp-service "25025"))
> +  (notmuch-search "subject:\"testing message sent via SMTP\"")
> +  (notmuch-test-wait)
> +  (notmuch-search-reply-to-thread)
> +  (end-of-buffer)
> +  (newline)
> +  (insert "Reply to a message via Emacs with fake SMTP")
> +  (message-send-and-exit))' >/dev/null 2>&1
> +wait ${smtp_dummy_pid}
> +notmuch new >/dev/null

The above part smells like CopyPasteProgramming; (old version of) 
emacs_deliver_message copied (old as this version is lacking
opportunistic quit). 

Maybe the functionality of emacs_deliver_message needs to be
split into 3 parts: initialization, sending and finalization.
then other uses can use initialization and finalization parts
and provide their own sending part.

I have work-in-progress code for initialization and finalization
parts of that and I don't want to do the changes in 2 places...

Tomi

Re: [PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Tomi Ollila]

On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet <pieter@praet.org> wrote:
> `notmuch-search-tag-all' (bound to "*") adds and removes tags
> to/from all messages which match the query used to populate the
> current search buffer.
> ---
>  test/emacs |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 

[ ... ]

> +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
> +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> +# Change tags of all matching messages and get tag counts
> +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")

Small nit: string equality comparison operator is '='. 
Use  grep '==' test/*  to confirm (or man test).

[ ... ]

Tomi

Re: [PATCH v2 1/4] test: emacs: rename subtest "Reply within emacs"

[Dmitry Kurochkin]

On Sun, 19 Feb 2012 21:38:25 +0100, Pieter Praet <pieter@praet.org> wrote:
> Rename subtest "Reply within emacs" to "notmuch-search: replying to a
> thread (composing)" as its title is misleading: it populates a reply
> buffer but doesn't submit it.
> ---

LGTM

Regards,
  Dmitry

>  test/emacs |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index b74cfa9..308d749 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -255,7 +255,7 @@ Subject:
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> -test_begin_subtest "Reply within emacs"
> +test_begin_subtest "notmuch-search: replying to a thread (composing)"
>  test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
>  	    (notmuch-test-wait)
>  	    (notmuch-search-reply-to-thread)
> -- 
> 1.7.8.1
> 

Re: [PATCH v2 3/4] test: emacs: new test "notmuch-search: when reply is sent, parent message should be tagged 'replied'"

[Dmitry Kurochkin]

On Sun, 19 Feb 2012 21:38:27 +0100, Pieter Praet <pieter@praet.org> wrote:
> When a message is replied to, it should be tagged `replied'.
> ---
>  test/emacs |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index 0f4f42b..b0fb760 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -314,6 +314,11 @@ Reply to a message via Emacs with fake SMTP
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "notmuch-search: when reply is sent, the parent message should be tagged 'replied'"
> +# depends on subtest "notmuch-search: replying to a thread (sending)"
> +output=$(notmuch search 'tag:replied' | notmuch_search_sanitize)

Add subject:"testing message sent via SMTP" to the query to avoid
potential collisions with other tests.

Regards,
  Dmitry

> +test_expect_equal "$output" "thread:XXX   2000-01-01 [1/2] Notmuch Test Suite; Testing message sent via SMTP (inbox replied)"
> +
>  test_begin_subtest "Quote MML tags in reply"
>  message_id='test-emacs-mml-quoting@message.id'
>  add_message [id]="$message_id" \
> -- 
> 1.7.8.1
> 

Re: [PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Dmitry Kurochkin]

On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet <pieter@praet.org> wrote:
> `notmuch-search-tag-all' (bound to "*") adds and removes tags
> to/from all messages which match the query used to populate the
> current search buffer.

LGTM.  But since you will need to send a new version to address Tomi's
comments anyway, below are few minor comments.

Regards,
  Dmitry

> ---
>  test/emacs |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index b0fb760..1db8540 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -124,6 +124,38 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
>  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
>  
> +test_begin_subtest "notmuch-search: change tags of all matching messages"
> +old_tag="inbox"
> +new_tag="xobni"
> +filter="AND from:cworth"
> +# Get initial tag counts and prevent false positives/negatives
> +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"

Consider s/1+/>0/.

> +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> +# Change tags of all matching messages and get tag counts
> +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
> +	    (notmuch-test-wait)
> +	    (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
> +old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
> +# Revert tag changes and get tag counts
> +test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
> +	    (notmuch-test-wait)
> +	    (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
> +old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
> +new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
> +# ... and verify the results
> +output="
> +before:   old:${old_tag_count_1} new:${new_tag_count_1}
> +after:    old:${old_tag_count_2} new:${new_tag_count_2}
> +restored: old:${old_tag_count_3} new:${new_tag_count_3}"
> +expected="
> +before:   old:${old_tag_count_1} new:0
> +after:    old:0 new:${old_tag_count_1}
> +restored: old:${old_tag_count_1} new:0"
> +test_expect_equal "$output" "$expected"
> +

I would add a newline before every commented block.

Regards,
  Dmitry

>  test_begin_subtest "Message with .. in Message-Id:"
>  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
>  test_emacs '(notmuch-search "id:\"123..456@example\"")
> -- 
> 1.7.8.1
> 

Re: [PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"

[Dmitry Kurochkin]

On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet <pieter@praet.org> wrote:
> As pointed out in the previous commit, the test for replying from
> within Emacs didn't actually submit the reply. This one does.
> ---
>  test/emacs |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index 308d749..0f4f42b 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
>  EOF
>  test_expect_equal_file OUTPUT EXPECTED
>  
> +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> +$TEST_DIRECTORY/smtp-dummy sent_message &
> +smtp_dummy_pid=$!
> +test_emacs \
> +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> +       (smtpmail-smtp-server "localhost")
> +       (smtpmail-smtp-service "25025"))
> +  (notmuch-search "subject:\"testing message sent via SMTP\"")
> +  (notmuch-test-wait)
> +  (notmuch-search-reply-to-thread)
> +  (end-of-buffer)
> +  (newline)
> +  (insert "Reply to a message via Emacs with fake SMTP")
> +  (message-send-and-exit))' >/dev/null 2>&1
> +wait ${smtp_dummy_pid}
> +notmuch new >/dev/null
> +sed \
> +    -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
> +    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' \
> +    -e s',^In-Reply-To: <.*>$,In-Reply-To: <XXX>,' \
> +    -e s',^References: <.*>$,References: <XXX>,' \
> +    -e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -0000,' < sent_message >OUTPUT

This sed command is a (partial) copy past from "Sending a message via
(fake) SMTP" test.  I suggest adding notmuch_sent_message_sanitize
function which would replace common headers with XXX.

Replace Date with XXX for Date for consistency with other headers,
AFAICT a valid date value is not needed here.

Regards,
  Dmitry

> +cat <<EOF >EXPECTED
> +From: Notmuch Test Suite <test_suite@notmuchmail.org>
> +To: user@example.com
> +Subject: Re: Testing message sent via SMTP
> +In-Reply-To: <XXX>
> +References: <XXX>
> +User-Agent: Notmuch/XXX Emacs/XXX
> +Date: Fri, 29 Mar 1974 10:05:00 -0000
> +Message-ID: <XXX>
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=us-ascii
> +
> +On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
> +> This is a test that messages are sent via SMTP
> +
> +Reply to a message via Emacs with fake SMTP
> +EOF
> +test_expect_equal_file OUTPUT EXPECTED
> +
>  test_begin_subtest "Quote MML tags in reply"
>  message_id='test-emacs-mml-quoting@message.id'
>  add_message [id]="$message_id" \
> -- 
> 1.7.8.1
> 

Re: [PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"

[Pieter Praet]

On Sun, 19 Feb 2012 23:04:38 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet <pieter@praet.org> wrote:
> > As pointed out in the previous commit, the test for replying from
> > within Emacs didn't actually submit the reply. This one does.
> > ---
> >  test/emacs |   41 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 41 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index 308d749..0f4f42b 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
> >  EOF
> >  test_expect_equal_file OUTPUT EXPECTED
> >  
> > +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> > +$TEST_DIRECTORY/smtp-dummy sent_message &
> > +smtp_dummy_pid=$!
> > +test_emacs \
> > +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> > +       (smtpmail-smtp-server "localhost")
> > +       (smtpmail-smtp-service "25025"))
> > +  (notmuch-search "subject:\"testing message sent via SMTP\"")
> > +  (notmuch-test-wait)
> > +  (notmuch-search-reply-to-thread)
> > +  (end-of-buffer)
> > +  (newline)
> > +  (insert "Reply to a message via Emacs with fake SMTP")
> > +  (message-send-and-exit))' >/dev/null 2>&1
> > +wait ${smtp_dummy_pid}
> > +notmuch new >/dev/null
> 
> The above part smells like CopyPasteProgramming; (old version of) 
> emacs_deliver_message copied (old as this version is lacking
> opportunistic quit). 
>

Nope, "Sending a message via (fake) SMTP", around 2011-05 to be exact [1].

My main intention was to get a bugfix [2] applied (still isn't BTW),
which might have been expedited by providing a test which checks
whether messages are properly tagged 'replied' when and *only* when
applicable, and for that I needed a quick way to test replying to a
thread, so I lazily repurposed an existing test.  Not much point in
reinventing the wheel, is there?

Still haven't gotten around to writing the actual test to demonstrate
that messages are tagged 'replied' prematurely, but this should be the
gist of it:

  #+begin_src emacs-lisp
    ;; ask for confirmation (using `y-or-n-p') before sending
    (let ((message-confirm-send t))
      ;; temporarily redefine `y-or-n-p', immediately returns nil
      (flet ((y-or-n-p (&rest args) nil))
        ;; then try to send message (which will fail)
        (message-send-and-exit)))

    ;; Parent message should NOT be tagged 'replied' now, yet it is,
    ;; since `notmuch-message-mark-replied' is added to `message-send-hook'
    ;; instead of `message-sent-hook'.
  #+end_src


> Maybe the functionality of emacs_deliver_message needs to be
> split into 3 parts: initialization, sending and finalization.
> then other uses can use initialization and finalization parts
> and provide their own sending part.
>

That would be *much* better, indeed.


> I have work-in-progress code for initialization and finalization
> parts of that and I don't want to do the changes in 2 places...
>

OK.  I'll mark this patch (and the next, which depends on it) as 'wip',
and pick them up again when your refactoring efforts hit mainline.


> Tomi


Peace

-- 
Pieter

[1] id:"1305299270-4316-1-git-send-email-pieter@praet.org"
[2] id:"1305275652-22956-1-git-send-email-pieter@praet.org"

Re: [PATCH v2 2/4] test: emacs: new test "notmuch-search: replying to a thread (sending)"

[Pieter Praet]

On Mon, 20 Feb 2012 16:44:32 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Sun, 19 Feb 2012 21:38:26 +0100, Pieter Praet <pieter@praet.org> wrote:
> > As pointed out in the previous commit, the test for replying from
> > within Emacs didn't actually submit the reply. This one does.
> > ---
> >  test/emacs |   41 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 41 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index 308d749..0f4f42b 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -273,6 +273,47 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
> >  EOF
> >  test_expect_equal_file OUTPUT EXPECTED
> >  
> > +test_begin_subtest "notmuch-search: replying to a thread (sending)"
> > +$TEST_DIRECTORY/smtp-dummy sent_message &
> > +smtp_dummy_pid=$!
> > +test_emacs \
> > +'(let ((message-send-mail-function '\''message-smtpmail-send-it)
> > +       (smtpmail-smtp-server "localhost")
> > +       (smtpmail-smtp-service "25025"))
> > +  (notmuch-search "subject:\"testing message sent via SMTP\"")
> > +  (notmuch-test-wait)
> > +  (notmuch-search-reply-to-thread)
> > +  (end-of-buffer)
> > +  (newline)
> > +  (insert "Reply to a message via Emacs with fake SMTP")
> > +  (message-send-and-exit))' >/dev/null 2>&1
> > +wait ${smtp_dummy_pid}
> > +notmuch new >/dev/null
> > +sed \
> > +    -e s',^User-Agent: Notmuch/.* Emacs/.*,User-Agent: Notmuch/XXX Emacs/XXX,' \
> > +    -e s',^Message-ID: <.*>$,Message-ID: <XXX>,' \
> > +    -e s',^In-Reply-To: <.*>$,In-Reply-To: <XXX>,' \
> > +    -e s',^References: <.*>$,References: <XXX>,' \
> > +    -e s',^Date: .*$,Date: Fri\, 29 Mar 1974 10:05:00 -0000,' < sent_message >OUTPUT
> 
> This sed command is a (partial) copy past from "Sending a message via
> (fake) SMTP" test.  [...]

Aye, that's the one.


> [...] I suggest adding notmuch_sent_message_sanitize
> function which would replace common headers with XXX.
> 
> Replace Date with XXX for Date for consistency with other headers,
> AFAICT a valid date value is not needed here.
>

Agreed.

Will have another look at it when Tomi's refactoring is done [1,2].


> Regards,
>   Dmitry
> 
> > +cat <<EOF >EXPECTED
> > +From: Notmuch Test Suite <test_suite@notmuchmail.org>
> > +To: user@example.com
> > +Subject: Re: Testing message sent via SMTP
> > +In-Reply-To: <XXX>
> > +References: <XXX>
> > +User-Agent: Notmuch/XXX Emacs/XXX
> > +Date: Fri, 29 Mar 1974 10:05:00 -0000
> > +Message-ID: <XXX>
> > +MIME-Version: 1.0
> > +Content-Type: text/plain; charset=us-ascii
> > +
> > +On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
> > +> This is a test that messages are sent via SMTP
> > +
> > +Reply to a message via Emacs with fake SMTP
> > +EOF
> > +test_expect_equal_file OUTPUT EXPECTED
> > +
> >  test_begin_subtest "Quote MML tags in reply"
> >  message_id='test-emacs-mml-quoting@message.id'
> >  add_message [id]="$message_id" \
> > -- 
> > 1.7.8.1
> > 


Peace

-- 
Pieter

[1] id:"m239a6wn4p.fsf@guru.guru-group.fi"
[2] id:"87mx8a900t.fsf@praet.org"

Re: [PATCH v2 3/4] test: emacs: new test "notmuch-search: when reply is sent, parent message should be tagged 'replied'"

[Pieter Praet]

On Mon, 20 Feb 2012 16:20:28 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Sun, 19 Feb 2012 21:38:27 +0100, Pieter Praet <pieter@praet.org> wrote:
> > When a message is replied to, it should be tagged `replied'.
> > ---
> >  test/emacs |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index 0f4f42b..b0fb760 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -314,6 +314,11 @@ Reply to a message via Emacs with fake SMTP
> >  EOF
> >  test_expect_equal_file OUTPUT EXPECTED
> >  
> > +test_begin_subtest "notmuch-search: when reply is sent, the parent message should be tagged 'replied'"
> > +# depends on subtest "notmuch-search: replying to a thread (sending)"
> > +output=$(notmuch search 'tag:replied' | notmuch_search_sanitize)
> 
> Add subject:"testing message sent via SMTP" to the query to avoid
> potential collisions with other tests.
>

Good point.

Depends on the previous test though, which is postponed [1],
so I won't submit a v3 of this for the time being.


> Regards,
>   Dmitry
> 
> > +test_expect_equal "$output" "thread:XXX   2000-01-01 [1/2] Notmuch Test Suite; Testing message sent via SMTP (inbox replied)"
> > +
> >  test_begin_subtest "Quote MML tags in reply"
> >  message_id='test-emacs-mml-quoting@message.id'
> >  add_message [id]="$message_id" \
> > -- 
> > 1.7.8.1
> > 


Peace

-- 
Pieter

[1] id:"87k43e8zxb.fsf@praet.org"

Re: [PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Pieter Praet]

On Sun, 19 Feb 2012 23:08:30 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet <pieter@praet.org> wrote:
> > `notmuch-search-tag-all' (bound to "*") adds and removes tags
> > to/from all messages which match the query used to populate the
> > current search buffer.
> > ---
> >  test/emacs |   32 ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> > 
> 
> [ ... ]
> 
> > +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> > +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> > +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
> > +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> > +# Change tags of all matching messages and get tag counts
> > +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
> 
> Small nit: string equality comparison operator is '='. 
> Use  grep '==' test/*  to confirm (or man test).
>

Argh...  Nice catch!


> [ ... ]
> 
> Tomi


Peace

-- 
Pieter

Re: [PATCH v2 4/4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Pieter Praet]

On Mon, 20 Feb 2012 16:25:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Sun, 19 Feb 2012 21:38:28 +0100, Pieter Praet <pieter@praet.org> wrote:
> > `notmuch-search-tag-all' (bound to "*") adds and removes tags
> > to/from all messages which match the query used to populate the
> > current search buffer.
> 
> LGTM.  But since you will need to send a new version to address Tomi's
> comments anyway, below are few minor comments.
> 
> Regards,
>   Dmitry
> 
> > ---
> >  test/emacs |   32 ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index b0fb760..1db8540 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -124,6 +124,38 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
> >  output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
> >  test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
> >  
> > +test_begin_subtest "notmuch-search: change tags of all matching messages"
> > +old_tag="inbox"
> > +new_tag="xobni"
> > +filter="AND from:cworth"
> > +# Get initial tag counts and prevent false positives/negatives
> > +old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
> > +new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
> > +test "${old_tag_count_1}" == "0" && old_tag_count_1="Need 1+ matches!"
> 
> Consider s/1+/>0/.
> 
> > +test "${new_tag_count_1}" == "0" || new_tag_count_1="Need 0 matches!"
> > +# Change tags of all matching messages and get tag counts
> > +test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
> > +	    (notmuch-test-wait)
> > +	    (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
> > +old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
> > +new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
> > +# Revert tag changes and get tag counts
> > +test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
> > +	    (notmuch-test-wait)
> > +	    (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
> > +old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
> > +new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
> > +# ... and verify the results
> > +output="
> > +before:   old:${old_tag_count_1} new:${new_tag_count_1}
> > +after:    old:${old_tag_count_2} new:${new_tag_count_2}
> > +restored: old:${old_tag_count_3} new:${new_tag_count_3}"
> > +expected="
> > +before:   old:${old_tag_count_1} new:0
> > +after:    old:0 new:${old_tag_count_1}
> > +restored: old:${old_tag_count_1} new:0"
> > +test_expect_equal "$output" "$expected"
> > +
> 
> I would add a newline before every commented block.
>

Agreed, on all accounts.


> Regards,
>   Dmitry
> 
> >  test_begin_subtest "Message with .. in Message-Id:"
> >  add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
> >  test_emacs '(notmuch-search "id:\"123..456@example\"")
> > -- 
> > 1.7.8.1
> > 


Peace

-- 
Pieter

[PATCH v3] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Pieter Praet]

`notmuch-search-tag-all' (bound to "*") adds and removes tags
to/from all messages which match the query used to populate the
current search buffer.
---
 test/emacs |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index b74cfa9..d64dcd0 100755
--- a/test/emacs
+++ b/test/emacs
@@ -124,6 +124,44 @@ test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
+
+test_begin_subtest "notmuch-search: change tags of all matching messages"
+old_tag="inbox"
+new_tag="xobni"
+filter="AND from:cworth"
+
+# Get initial tag counts and prevent false positives/negatives
+old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
+test "${old_tag_count_1}" = "0" && old_tag_count_1="Need >0 matches!"
+test "${new_tag_count_1}" = "0" || new_tag_count_1="Need 0 matches!"
+
+# Change tags of all matching messages and get tag counts
+test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
+old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
+
+# Revert tag changes and get tag counts
+test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
+old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
+
+# ... and verify the results
+output="
+before:   old:${old_tag_count_1} new:${new_tag_count_1}
+after:    old:${old_tag_count_2} new:${new_tag_count_2}
+restored: old:${old_tag_count_3} new:${new_tag_count_3}"
+expected="
+before:   old:${old_tag_count_1} new:0
+after:    old:0 new:${old_tag_count_1}
+restored: old:${old_tag_count_1} new:0"
+test_expect_equal "$output" "$expected"
+
+
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
 test_emacs '(notmuch-search "id:\"123..456@example\"")
-- 
1.7.8.1

[PATCH v4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Pieter Praet]

* test/emacs-search:

  - New subtest "notmuch-search: change tags of all matching messages":
    `notmuch-search-tag-all' (bound to "*") adds and removes tags
    to/from all messages which match the query used to populate the
    current search buffer.
---

Moved to a separate file to prevent a merge conflict when applied in
conjunction with the patches listed @ id:"877gz4dnl2.fsf@praet.org".


 test/emacs-search |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 49 insertions(+), 0 deletions(-)
 create mode 100755 test/emacs-search

diff --git a/test/emacs-search b/test/emacs-search
new file mode 100755
index 0000000..1cd8a89
--- /dev/null
+++ b/test/emacs-search
@@ -0,0 +1,48 @@
+#!/usr/bin/env bash
+
+test_description="Emacs notmuch-search-mode"
+. test-lib.sh
+
+EXPECTED=$TEST_DIRECTORY/emacs-show.expected-output
+
+add_email_corpus
+
+
+test_begin_subtest "notmuch-search: change tags of all matching messages"
+old_tag="inbox"
+new_tag="xobni"
+filter="AND from:cworth"
+
+# Get initial tag counts and prevent false positives/negatives
+old_tag_count_1=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_1=$(notmuch count tag:"${new_tag}" "${filter}")
+test "${old_tag_count_1}" = "0" && old_tag_count_1="Need >0 matches!"
+test "${new_tag_count_1}" = "0" || new_tag_count_1="Need 0 matches!"
+
+# Change tags of all matching messages and get tag counts
+test_emacs "(notmuch-search \"tag:${old_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"-${old_tag}\" \"+${new_tag}\")"
+old_tag_count_2=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_2=$(notmuch count tag:"${new_tag}" "${filter}")
+
+# Revert tag changes and get tag counts
+test_emacs "(notmuch-search \"tag:${new_tag} ${filter}\")
+	    (notmuch-test-wait)
+	    (notmuch-search-tag-all \"+${old_tag}\" \"-${new_tag}\")"
+old_tag_count_3=$(notmuch count tag:"${old_tag}" "${filter}")
+new_tag_count_3=$(notmuch count tag:"${new_tag}" "${filter}")
+
+# ... and verify the results
+output="
+before:   old:${old_tag_count_1} new:${new_tag_count_1}
+after:    old:${old_tag_count_2} new:${new_tag_count_2}
+restored: old:${old_tag_count_3} new:${new_tag_count_3}"
+expected="
+before:   old:${old_tag_count_1} new:0
+after:    old:0 new:${old_tag_count_1}
+restored: old:${old_tag_count_1} new:0"
+test_expect_equal "$output" "$expected"
+
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index e14d34e..81d0d7f 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -54,6 +54,7 @@ TESTS="
   argument-parsing
   emacs-test-functions
   emacs-address-cleaning
+  emacs-search
   emacs-show
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
-- 
1.7.8.1

Re: [PATCH v4] test: emacs: new test "notmuch-search: change tags of all matching messages"

[Tomi Ollila]

On Wed,  7 Mar 2012 20:49:52 +0100, Pieter Praet <pieter@praet.org> wrote:
> * test/emacs-search:
> 
>   - New subtest "notmuch-search: change tags of all matching messages":
>     `notmuch-search-tag-all' (bound to "*") adds and removes tags
>     to/from all messages which match the query used to populate the
>     current search buffer.
> ---
> 
> Moved to a separate file to prevent a merge conflict when applied in
> conjunction with the patches listed @ id:"877gz4dnl2.fsf@praet.org".

Hi Pieter!

I did the following script:

--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--

#!/bin/sh

while read line
do
        case $line in id:*) notmuch show --format=mbox "$line"
        esac
done < "$0" | git am 
exit 0

id:"1329684990-12504-2-git-send-email-pieter@praet.org"
id:"1329684990-12504-3-git-send-email-pieter@praet.org"
id:"1329684990-12504-4-git-send-email-pieter@praet.org"

id:"1330122640-18895-2-git-send-email-pieter@praet.org"
id:"1330122640-18895-3-git-send-email-pieter@praet.org"

id:"1331149792-17192-1-git-send-email-pieter@praet.org"

--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--

When executed on top of current master (c46764030d, 0.11.1-1-293-gc467640)
last patch (1331149792-17192-1..., this I'm replying to) fails as:

error: patch failed: test/notmuch-test:54
error: test/notmuch-test: patch does not apply


In order to get clear picture of the changes you wish to get
reviewed please provide cleanly applicable patchset.

The best possible way to inform what is to be reviewed
is

1) List of message id:s of the patches, in order those
   are to be applied.

2) Sha1 of the commit where this patchset applies cleanly
   (In this case I will do review even though it would not
    apply on top of current master (at that time) anymore --
    but *I* would not expect it to to be pushed until rebased
    patchset is sent again).

BR,

Tomi