From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: notmuch@notmuchmail.org
Subject: Re: [PATCH 1/3] test: `notmuch-show-advance-and-archive' with
	invisible signature
In-Reply-To: <87pqlxw7d0.fsf@gmail.com>
References: <1309312132-14564-1-git-send-email-dmitry.kurochkin@gmail.com>
	<87pqlxw7d0.fsf@gmail.com>
User-Agent: Notmuch/0.5-297-gfb148d8 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Wed, 29 Jun 2011 23:57:24 +0400
Message-ID: <87tyb8v2aj.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Wed, 29 Jun 2011 09:10:19 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Wed, 29 Jun 2011 05:48:50 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Add Emacs test to check that `notmuch-show-advance-and-archive'
> > works for the last message in thread with invisible signature.
> > ---
> > 
> > This patch series fixes the bug reported by Sebastien in [1].  I
> > was able to reproduce it and confirm that the second patch from
> > this series fixes the problem.  Unfortunately, I can not explain
> > why it fixes it.  The patch uses a cleaner approach for visible
> > text search.  But the old approach should work fine as well.
> > Apparently, it does not work when `invisible' property is not a
> > single symbol but a list (which was changed in
> > 95ef8da29439f2e79115c36ab4d2a80aef1a1462).  I suspect that it is
> > an Emacs bug.  I plan to look at it later.
> > 
> 
> Turns out that `point-invisible-p' is a function from notmuch-lib.el, I
> did not realize that before.  It implements a custom visibility check
> which is incomplete and does not work correctly when `invisible'
> property is a list.  That is why the previous code (which used
> `point-invisible-p') had the bug.  I sent another patch that removes
> `point-invisible-p' function.
> 
> > Another issue is that the test does not demonstrate the bug.
> > Again, I do not really know why.  It passes both before and after
> > the fix.  Although if I run the test commands by hand I hit the
> > bug.  I guess it has something to do with emacs daemon mode when
> > the buffer is not visible.  I hope someone with a better elisp
> > knowledge can tell what is going on and how to make the test
> > work.
> > 
> 
> Now it is clear where the bug was.  Remaining question is how to test
> it.
> 

I have posted a new version of this patch series [1].  It fixes the test
to actually demonstrate the bug.

Regards,
  Dmitry

[1] id:"1309376558-26284-1-git-send-email-dmitry.kurochkin@gmail.com"

> Regards,
>   Dmitry
> 
> > I believe patches 2 and 3 can be pushed after review even without
> > a working test.
> > 
> > Regards,
> >   Dmitry
> > 
> > [1] id:"8739j5rn2d.fsf@cern.ch"
> > 
> >  test/emacs |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/test/emacs b/test/emacs
> > index e59de47..65a96a5 100755
> > --- a/test/emacs
> > +++ b/test/emacs
> > @@ -347,4 +347,16 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
> >  	    (test-visible-output)'
> >  test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
> >  
> > +test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
> > +message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
> > +message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
> > +test_emacs "(notmuch-search \"$message1 or $message2\")
> > +	    (notmuch-test-wait)
> > +	    (notmuch-search-show-thread)
> > +	    (notmuch-show-advance-and-archive)
> > +	    (test-output)"
> > +test_emacs "(notmuch-show \"$message2\")
> > +	    (test-output \"EXPECTED\")"
> > +test_expect_equal_file OUTPUT EXPECTED
> > +
> >  test_done
> > -- 
> > 1.7.5.4
> >