From: Thomas Jost <schnouki@schnouki.net>
To: Jameson Graef Rollins <jrollins@finestructure.net>,
notmuch@notmuchmail.org
Subject: Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
Date: Wed, 16 Nov 2011 21:17:48 +0100 [thread overview]
Message-ID: <87zkfv95zn.fsf@schnouki.net> (raw)
In-Reply-To: <87lirf99vt.fsf@servo.finestructure.net>
[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]
On Wed, 16 Nov 2011 10:53:42 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> > Hello list,
> >
> > This is another rebased version of Pieter's series to add GPG and Emacs as test
> > prereqs, plus some additions on my own. (Rebased and posted as requested by
> > Pieter [1].)
> >
> > Changes as compared to Pieter's patches (including parts from [2]):
> > - prereqs are not tested using test_expect_success as they were in Pieter's
> > original patches, but using a new function called test_set_bin_prereq. I wrote
> > this before the gdb prereq was added, hence the different way to set it.
>
> Hey, Thomas. Thanks so much for this work. This sounds like a better
> solution.
>
> However, in the patches you send I see a lot of changes of the form
>
> -test_expect_success 'emacs delivery of encrypted message with attachment' \
> +test_expect_success GPG 'emacs delivery of encrypted message with attachment' \
>
> and
>
> -test_expect_equal \
> +test_expect_equal GPG \
>
> which seems to contradict what you've said above. Not to mention that I
> don't see anything that modifies calls to the test_expect_ functions.
> Basically I see a lot more in the diffs than I would have expected in a
> cursory look. Is this just a rebase flub, or is there something I'm
> missing?
>
> jamie.
Hi Jamie,
I guess I wasn't clear in my explanations :)
Pieter's patches use this to detect the presence of GPG/Emacs and set
the prereq:
+# GnuPG is a prereq.
+test_expect_success "prereq: GnuPG is present" "which gpg" \
+ && test_set_prereq GPG
There are 2 problems with this approach:
- test_expect_success returns 0 regardless of the actual result of the
command it runs. So even if gpg is not installed,
text_expect_success "..." "which gpg"
will succeed, and "test_set_prereq GPG" will be run.
This, however, has been fixed in commit 003e7180 -- which had not been
pushed when I wrote this in the first place :)
- using test_expect_* to set a prereq does not make sense. If emacs is
absent, the test suite would report a failed test. But a missing
prereq is *not* a notmuch issue, so this should *not* be reported as a
failed test.
Hence my first patch, which defines test_set_bin_prereq, a new helper
function to set a prereq without using any test_expect_*.
After that we can use the normal prereq syntax from the test suite:
- test_expect_success COMMAND --> run COMMAND, expecting it to succeed
- test_expect_success PREREQ COMMAND --> skip if PREREQ is not set, else
run the test as before
(and same thing with the other test_expect_* functions)
Does it make more sense now?
Regards,
--
Thomas/Schnouki
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
next prev parent reply other threads:[~2011-11-16 20:17 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-01 19:49 set test prereqs (Emacs, GDB, GPG) v4 Pieter Praet
2011-11-01 19:49 ` [PATCH 1/4] test: add 'GnuPG' prereq to dependent 'crypto' tests Pieter Praet
2011-11-01 21:17 ` Jameson Graef Rollins
2011-11-02 17:20 ` Pieter Praet
2011-11-01 19:49 ` [PATCH 2/4] test: add 'Emacs' " Pieter Praet
2011-11-01 19:49 ` [PATCH 3/4] test: add 'Emacs' prereq to dependent 'emacs' tests Pieter Praet
2011-11-01 19:49 ` [PATCH 4/4] test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer' tests Pieter Praet
2011-11-01 20:20 ` set test prereqs (Emacs, GDB, GPG) v4 Ali Polatel
2011-11-02 17:21 ` Pieter Praet
2011-11-16 14:33 ` [PATCH 0/6] Rebase of Pieter's "set test prereqs" Thomas Jost
2011-11-16 14:33 ` [PATCH 1/6] test: define a helper function for defining prereqs on executables Thomas Jost
2011-11-16 14:33 ` [PATCH 2/6] test: check if emacs and dtach are available in test_emacs() Thomas Jost
2011-11-16 14:33 ` [PATCH 3/6] test: add 'GnuPG' prereq to dependent 'crypto' tests Thomas Jost
2011-11-16 14:33 ` [PATCH 4/6] test: add 'Emacs' " Thomas Jost
2011-11-16 14:33 ` [PATCH 5/6] test: add 'Emacs' prereq to dependent 'emacs' tests Thomas Jost
2011-11-16 14:33 ` [PATCH 6/6] test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer' tests Thomas Jost
2011-11-16 18:53 ` [PATCH 0/6] Rebase of Pieter's "set test prereqs" Jameson Graef Rollins
2011-11-16 20:17 ` Thomas Jost [this message]
2011-11-16 20:50 ` Pieter Praet
2011-11-17 10:14 ` Thomas Jost
2012-01-12 17:07 ` Pieter Praet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zkfv95zn.fsf@schnouki.net \
--to=schnouki@schnouki.net \
--cc=jrollins@finestructure.net \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).