From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 1C6A16DE0F4F for ; Thu, 11 Apr 2019 04:27:01 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -2.698 X-Spam-Level: X-Spam-Status: No, score=-2.698 tagged_above=-999 required=5 tests=[AWL=-0.196, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cRYWfS-joXtK for ; Thu, 11 Apr 2019 04:26:59 -0700 (PDT) Received: from smtp-4.sys.kth.se (smtp-4.sys.kth.se [130.237.48.193]) by arlo.cworth.org (Postfix) with ESMTPS id 58EA76DE0F31 for ; Thu, 11 Apr 2019 04:26:59 -0700 (PDT) Received: from smtp-4.sys.kth.se (localhost.localdomain [127.0.0.1]) by smtp-4.sys.kth.se (Postfix) with ESMTP id 040F02B33; Thu, 11 Apr 2019 13:26:56 +0200 (CEST) X-Virus-Scanned: by amavisd-new at kth.se Received: from smtp-4.sys.kth.se ([127.0.0.1]) by smtp-4.sys.kth.se (smtp-4.sys.kth.se [127.0.0.1]) (amavisd-new, port 10024) with LMTP id VPU_qCkD71i4; Thu, 11 Apr 2019 13:26:54 +0200 (CEST) X-KTH-Auth: ekeberg [2001:6b0:1:1de0:61f4:9ef7:6759:4efe] DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kth.se; s=default; t=1554982014; bh=7HiBu+UgwBS4HlqbpRq1fkCPndT7nEQpVq+2mqCt8JY=; h=From:To:Subject:In-Reply-To:References:Date; b=Qi1FsRu9CVXYuu3yV3LfambIEaS4n5AqBFNJfSFnL4hmlGVhJwBpMSWbl9GcIwLEf Zefqxl08dAqwcQN8mYlZ0d/50Cp7sXWMeFdiqP98il2XpEUV6FdtVoZujaQV2DzfzK ZMcqxYNFWCn3R6S6xhcvG2MEVr/5kn4v2mROQ5dk= X-KTH-mail-from: ekeberg@kth.se Received: from swing (unknown [IPv6:2001:6b0:1:1de0:61f4:9ef7:6759:4efe]) by smtp-4.sys.kth.se (Postfix) with ESMTPSA id A30211C5D; Thu, 11 Apr 2019 13:26:53 +0200 (CEST) From: =?utf-8?Q?=C3=96rjan?= Ekeberg To: Tomi Ollila , notmuch@notmuchmail.org Subject: Re: [PATCH v3 4/4] test: add test for checking forwarded messages In-Reply-To: References: <20190404230126.4283-1-ekeberg@kth.se> <20190410121908.5998-1-ekeberg@kth.se> <20190410121908.5998-2-ekeberg@kth.se> Date: Thu, 11 Apr 2019 13:26:53 +0200 Message-ID: <87r2a83hqa.fsf@swing.csc.kth.se> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Apr 2019 11:27:01 -0000 Tomi Ollila 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@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 f= orwarded-tag >> + >> +test_expect_equal $(notmuch search --output=3Dmessages tag:forwarded) i= d:$message_id > > I'd still do something like: > > test_expect_equal "$(notmuch search --output=3Dtags id:$message_id)" \ > $'forwarded\ninbox\nunread\n'=20 > > (note: quoting aroung first arg and $'dollar-single' in second, but did n= ot > 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=3Dtags id:$message_id | sort)"= \ "forwarded inbox unread" /=C3=96rjan