From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Pieter Praet <pieter@praet.org>
Subject: Re: [PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
In-Reply-To: <1327909550-6788-1-git-send-email-pieter@praet.org>
References: <1327599289-16865-1-git-send-email-dmitry.kurochkin@gmail.com>
	<1327909550-6788-1-git-send-email-pieter@praet.org>
User-Agent: Notmuch/0.11+139~gd9b7cab (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Mon, 30 Jan 2012 12:13:48 +0400
Message-ID: <87y5spober.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Notmuch Mail <notmuch@notmuchmail.org>
Precedence: list

Hi Pieter.

On Mon, 30 Jan 2012 08:45:50 +0100, Pieter Praet <pieter@praet.org> wrote:
> `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.
> 
> ---
> 
> Rebased to current master.
> 
> Previous versions (chronologically):
> - id:"1309450108-2793-1-git-send-email-pieter@praet.org"
> - id:"1309762318-4530-5-git-send-email-pieter@praet.org"
> - id:"1310313335-4159-5-git-send-email-pieter@praet.org"
> 

This looks like a useful patch series.  We definitely need more tests
for tagging operations in the Emacs UI.  Do you plan to revive it?

Note that not so long ago I posted a bunch of tagging-related patches
[1] that would conflict with this patch at least because of
`notmuch-search-operate-all' being renamed to `notmuch-search-tag-all'.

> 
>  test/emacs |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/test/emacs b/test/emacs
> index 8ca4c8a..e94ad94 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -124,6 +124,25 @@ 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 "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

I am worried that this test would break because of changes in other
tests.  E.g. if a new test adds a new message which matches "tag:inbox
AND tags", this test would have to be updated.  I think we should avoid
this.  I see the following options here:

* Search for messages which are less likely to change, e.g. "from:carl".

* Rework the test to avoid using any fixed expected results, e.g.:

  - count all messages with tag:inbox

  - remove inbox tag, add some other distinct tag for all messages with
    tag:inbox

  - count all messages with tag:inbox again, check that it is 0

  - add the inbox tag back, remove the previously added tag, check the
    message count

I like the latter approach because it does not compare Emacs UI output
and hence would not break when that output changes.  What do you think?

Also, we should leave notmuch db in the same state as it was before the
test if possible.

Regards,
  Dmitry

[1] id:"1327901644-15799-1-git-send-email-dmitry.kurochkin@gmail.com"

> +
>  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
>