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