[PATCH v4] test: emacs: add test for `notmuch-search-operate-all'
Pieter Praet
pieter at praet.org
Sun Feb 19 12:42:46 PST 2012
On Thu, 02 Feb 2012 03:25:40 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > > [...]
> >
> > OK, how about this?:
> >
>
> Looks good. Minor comments below.
>
> > #+begin_src sh
> > test_begin_subtest "Add/remove tags to/from all matching messages."
>
> We should add notmuch-search here, because similar functionality would
> be available in notmuch-show soon. Also, I suggest replacing
> "add/remove tags to/from" with "change tags". Consider:
>
> Change tags for all matching threads in notmuch-search.
>
Agreed, though I think you meant "matching messages".
Although, Austin has suggested [1] having `notmuch-search-operate-all'
(s/operate/tag) work on threads (or all messages therein, at least),
as opposed to only on *matched* messages, with which I agree(d) [2].
This would also get rid of the regexp issue @ [3], allowing us to
fix the race condition without having to put `notmuch-search' on a
JSON diet first.
And OTOH, David has asked [4] for `notmuch-show-tag-all' to work only on
visible (uncollapsed) messages, so wrt keeping `notmuch-search-tag-all'
and `notmuch-show-tag-all' symmetrical, it seems we're at an impasse :)
> > old="inbox" ; new="xobni" ; filter="AND from:cworth"
>
> I would prefer this to be on separate lines.
>
> > o1=$(notmuch count tag:"${old}" "${filter}") ; n1=$(notmuch count tag:"${new}" "${filter}")
>
> I would definately prefer this to be on separate lines.
>
> Also, please consider s/o1/old_count_1, s/n1/new_count_1/ and so on.
>
> > test "${o1}" == "0" && o1="Need more matches!" # prevent false positives
> > test_emacs "(notmuch-search \"tag:${old} ${filter}\")
> > (notmuch-test-wait)
> > (notmuch-search-operate-all \"+${new}\" \"-${old}\")"
> > o2=$(notmuch count tag:"${old}" "${filter}") ; n2=$(notmuch count tag:"${new}" "${filter}")
>
> Same comment about separate lines.
>
> > notmuch tag -"${new}" +"${old}" -- tag:"${new}" "${filter}" AND NOT tag:"${old}"} # restore db state!
>
> Why "AND NOT tag:$old" is needed here?
>
To be extra sure we're only touching the messages which were altered in
the previous segment, but it's probably rather useless. Removed.
> Since the line is too long, please move the comment to a separate line
> above.
>
> Since we are testing Emacs UI, should we restore db though Emacs as
> well?
>
Hmm, comes with a performance hit (which is why I avoided it initially),
but a fairly minor one at that. Done.
> > o3=$(notmuch count tag:"${old}" "${filter}") ; n3=$(notmuch count tag:"${new}" "${filter}")
>
> Same comment about separate lines.
>
> > output="
> > before: ${old}=$o1 ${new}=$n1
> > after: ${old}=$o2 ${new}=$n2
> > restored: ${old}=$o3 ${new}=$n3"
> > expected="
> > before: ${old}=$o1 ${new}=0
> > after: ${old}=0 ${new}=$o1
> > restored: ${old}=$o1 ${new}=0"
>
> I would change "=" to ":".
>
Done, as well as everything related to separating lines and
renaming variables.
Fresh patch submitted in its original thread [5].
> Regards,
> Dmitry
>
> > [...]
Peace
--
Pieter
[1] id:"20111112163502.GE2658 at mit.edu"
[2] id:"871ut8f9ya.fsf at praet.org"
[3] id:"1310416993-31031-1-git-send-email-pieter at praet.org"
[4] id:"87wr7xqpuf.fsf at rocinante.cs.unb.ca"
[5] id:"1329683908-5435-1-git-send-email-pieter at praet.org"
More information about the notmuch
mailing list