Re: [PATCH 3/4 v2] test: Add test for Original Message hiding at point-min.

[Dmitry Kurochkin <dmitry.kurochkin@gmail.com>]

On Wed, 25 Jan 2012 15:45:27 +0000, David Edmondson <dme@dme.org> wrote:
> ---
>  test/emacs-original-message-hiding.el |   15 +++++++++++++++
>  test/emacs-original-message-hiding.sh |   20 ++++++++++++++++++++
>  test/notmuch-test                     |    1 +
>  3 files changed, 36 insertions(+), 0 deletions(-)
>  create mode 100644 test/emacs-original-message-hiding.el
>  create mode 100755 test/emacs-original-message-hiding.sh
> 
> diff --git a/test/emacs-original-message-hiding.el b/test/emacs-original-message-hiding.el
> new file mode 100644
> index 0000000..bbacbeb
> --- /dev/null
> +++ b/test/emacs-original-message-hiding.el
> @@ -0,0 +1,15 @@
> +(defun notmuch-test-original-message-hiding ()
> +  (let ((notmuch-show-insert-text/plain-hook '(notmuch-wash-excerpt-citations))

Do we have to override the default value here?  I thought
notmuch-wash-excerpt-citations was enabled by default.

> +	(expected "\
> +Sender <sender@example.com> (2010-01-05) (inbox)
> +Subject: hiding an Original Message
> +To: mailinglist@notmuchmail.org
> +Date: Tue, 05 Jan 2010 15:43:56 -0000
> +
> +[ 2-line hidden original message. Click/Enter to show. ]
> +
> +")
> +	output)
> +    (notmuch-show "id:\"test_message@test.org\"")
> +    (setq output (visible-buffer-string))
> +    (notmuch-test-expect-equal output expected)))

IMO writing the test in lisp does not give any benefit in this case.
Quite the opposite: a simple test is split in two files and becomes more
complex.

But since we accepted this way of writing tests and you seem to prefer
it, I would not argue about changing it.

> diff --git a/test/emacs-original-message-hiding.sh b/test/emacs-original-message-hiding.sh
> new file mode 100755
> index 0000000..01cf98d
> --- /dev/null
> +++ b/test/emacs-original-message-hiding.sh

Let's rename this file to something a bit more general, e.g.
emacs-message-hiding.  Then we can put new tests (and move all
existing!) related to hiding message parts into it.

> @@ -0,0 +1,20 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs Original Message hiding"
> +. test-lib.sh
> +
> +test_begin_subtest "Hiding an Original Message region at point-min"
> +add_message \
> +    '[id]="test_message@test.org"' \

The message-id should be more unique to avoid collisions with other
tests.  IMO deriving message-id from the test description is a good
practice.  Also, consider assigning it to a variable and using it
instead of the string.  Since part of the test is in lisp, it is tricky
to use this variable in it.  Perhaps we should pass it as a parameter to
notmuch-test-original-message-hiding?  Or just do not bother and use
string constants :)  Your choice.

Regards,
  Dmitry

> +    '[from]="Sender <sender@example.com>"' \
> +    '[to]=mailinglist@notmuchmail.org' \
> +    '[subject]="hiding an Original Message"' \
> +    '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
> +    '[body]="-----Original Message-----
> +Text here.
> +"'
> +test_subtest_known_broken
> +test_emacs_expect_t \
> +    '(load "emacs-original-message-hiding.el") (notmuch-test-original-message-hiding)'
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index 3f1740c..af132fc 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -54,6 +54,7 @@ TESTS="
>    argument-parsing
>    emacs-test-functions.sh
>    emacs-address-cleaning.sh
> +  emacs-original-message-hiding.sh
>  "
>  TESTS=${NOTMUCH_TESTS:=$TESTS}
>  
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch