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

[Örjan Ekeberg]

Tomi Ollila <tomi.ollila at iki.fi> writes:
>
> good stuff -- comments inline

Yes, the test suite in general could benefit from more comments, since
there are quite a lot of "smart things" going on.

>> +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
>
> ?

I agree that it is more straightforward to call notmuch-mua-send than to
only call the send hooks.  Thanks for your tip on how to make this work.
In addition to your suggestion, I had to remove the automatically added
Fcc-header and add a proper To-address for the send machinery to accept
the message.  This seems to work:

test_emacs_expect_t "
  (let ((message-send-mail-function (lambda () t)))
    (notmuch-show \"id:$message_id\")
    (notmuch-show-forward-message)
    (save-restriction
      (message-narrow-to-headers)
      (message-remove-header \"Fcc\")
      (message-remove-header \"To\")
      (message-add-header \"To: nobody at example.com\"))

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

>> +    (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

Ok.


>> +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...

Fair enough.  I agree that it is more logical to analyse the tags of the
specific message, than to first search for the tag itself.

One worry though.  Your suggested code depends on that the order in
which "notmuch search" lists the tags is stable.  Since the tags is a
set, it seems fragile to rely on the order of the individual tags.  What
about this variant?  I have tried it and it seems to work.

test_expect_equal "$(notmuch search --output=tags id:$message_id | sort)" \
"forwarded
inbox
unread"

/Örjan