Re: [PATCH v3 4/4] test: add test for checking forwarded messages

Subject: Re: [PATCH v3 4/4] test: add test for checking forwarded messages

Date: Wed, 10 Apr 2019 23:25:05 +0300

To: Örjan Ekeberg, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


On Wed, Apr 10 2019, Örjan Ekeberg wrote:

> Add test of forwarding messages from within emacs.
> The first test checks that a references header is properly
> added to the new message.  The second test checks that the
> send-hook of the forwarding message adds a forwarded-tag
> to the original message.

good stuff -- comments inline

> ---
>  test/T730-emacs-forwarding.sh | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100755 test/T730-emacs-forwarding.sh
>
> diff --git a/test/T730-emacs-forwarding.sh b/test/T730-emacs-forwarding.sh
> new file mode 100755
> index 00000000..d1a573fb
> --- /dev/null
> +++ b/test/T730-emacs-forwarding.sh
> @@ -0,0 +1,30 @@
> +#!/usr/bin/env bash
> +
> +test_description="emacs forwarding"
> +. $(dirname "$0")/test-lib.sh || exit 1
> +
> +test_begin_subtest "Forward setting the correct references header"
> +# Check that, when forwarding a message, the new message has
> +# a References-header pointing to the original (forwarded) message.
> +
> +message_id='OriginalMessage@notmuchmail.org'
> +add_message \
> +    [id]="$message_id" \
> +    '[from]="user@example.com"' \
> +    '[subject]="This is the original message"' \
> +    '[body]="Dummy text."'
> +
> +test_emacs_expect_t "
> +   (progn
> +    (notmuch-show \"id:$message_id\")
> +    (notmuch-show-forward-message)
> +    (run-hooks 'notmuch-mua-send-hook)

Would instead (run-hooks...) in the above, embedding:

    (let ((message-send-mail-function (lambda () t))
          (mail-host-address \"example.com\"))
and
     (notmuch-mua-send)

work (that is what we do in emacs_fcc_message () in test-lib.sh

?

> +    (notmuch-test-expect-equal (message-field-value \"References\")
> +                               \"<$message_id>\"))"

You probably had tabs and spaces "correct" last time, but I got confused by
the extra line and forgot to drop indentation to see better. To be
consistent with other rests these lines do need to have tabs (width 8) and
rest spaces

> +
> +test_begin_subtest "Forwarding adding the forwarded tag"
> +# Check that the send hook called in the previous subtest did add the forwarded-tag
> +
> +test_expect_equal $(notmuch search --output=messages tag:forwarded) id:$message_id

I'd still do something like:

 test_expect_equal "$(notmuch search --output=tags id:$message_id)" \
 	$'forwarded\ninbox\nunread\n' 

(note: quoting aroung first arg and $'dollar-single' in second, but did not
test that this is what exactly works (newlines could be clearer, though) :)

... to ensure (and show visibility) that just "forwarded" were added. if
anyone(tm) ever changes system so that default tags change, many other
tests also need updating...

> +
> +test_done
> -- 
> 2.20.1
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: