Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
On Thu, 17 Nov 2011 11:14:11 +0100, Thomas Jost wrote: > On Wed, 16 Nov 2011 21:50:17 +0100, Pieter Praet wrote: > > On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost > > 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].) > > > > > > > Thanks Thomas! > > > > Although... you may have misread (or maybe I mistyped :), but what I > > actually intended [1] was for you to rebase *only* your fixes on top of > > my rebased series (e.g. see "tjost-fixes.patch" in att), so you could > > receive proper credit for cleaning up my mess. > > Oh, ok, I must have misread that :) > > Right now your patches don't apply cleanly on master (conflict in patch > 3 due to commit 5964a7), and I think that Dmitry's patches [1] may be a > better way to handle prereqs. So I probably won't send those patches > until we decide which approach is the way to go. > > [1] id:"1321494986-18998-1-git-send-email-dmitry.kuroch...@gmail.com" > Yup, Dmitry's solution is much more elegant! > > Also, while my apprehension [2,3] re the inclusion of the SCREEN/DTACH > > prereq in patches #4,5,6 didn't have much merit (it's an all-or-nothing > > affair anyways), the issue [3] in patch #5 @ "Reply within emacs" still > > stands: `sed' will run unconditionally, and treat "EMACS" as an input > > file. (see "sed-prereq-fix.patch" in att). > > Nice catch with this sed issue. Looks like I need to be more careful > when replacing "OUTPUT" with "EMACS OUTPUT"... > > Thanks, > > -- > Thomas/Schnouki Peace -- Pieter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
On Wed, 16 Nov 2011 21:50:17 +0100, Pieter Praet wrote: > On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost 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].) > > > > Thanks Thomas! > > Although... you may have misread (or maybe I mistyped :), but what I > actually intended [1] was for you to rebase *only* your fixes on top of > my rebased series (e.g. see "tjost-fixes.patch" in att), so you could > receive proper credit for cleaning up my mess. Oh, ok, I must have misread that :) Right now your patches don't apply cleanly on master (conflict in patch 3 due to commit 5964a7), and I think that Dmitry's patches [1] may be a better way to handle prereqs. So I probably won't send those patches until we decide which approach is the way to go. [1] id:"1321494986-18998-1-git-send-email-dmitry.kuroch...@gmail.com" > Also, while my apprehension [2,3] re the inclusion of the SCREEN/DTACH > prereq in patches #4,5,6 didn't have much merit (it's an all-or-nothing > affair anyways), the issue [3] in patch #5 @ "Reply within emacs" still > stands: `sed' will run unconditionally, and treat "EMACS" as an input > file. (see "sed-prereq-fix.patch" in att). Nice catch with this sed issue. Looks like I need to be more careful when replacing "OUTPUT" with "EMACS OUTPUT"... Thanks, -- Thomas/Schnouki pgpsE3YkxBLkn.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost 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].) > Thanks Thomas! Although... you may have misread (or maybe I mistyped :), but what I actually intended [1] was for you to rebase *only* your fixes on top of my rebased series (e.g. see "tjost-fixes.patch" in att), so you could receive proper credit for cleaning up my mess. Also, while my apprehension [2,3] re the inclusion of the SCREEN/DTACH prereq in patches #4,5,6 didn't have much merit (it's an all-or-nothing affair anyways), the issue [3] in patch #5 @ "Reply within emacs" still stands: `sed' will run unconditionally, and treat "EMACS" as an input file. (see "sed-prereq-fix.patch" in att). Thanks again! > 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. > > - some fixes in Pieter's patches so that it actually works when gpg is not > installed. Can't exactly remember what (...but you can just check his > original > patches), but in the end it was working fine in a chroot without gpg. > > - since Emacs is now run using dtach, the emacs prereq also depends on dtach. > The presence of emacs and dtach is also checked in the test_emacs() function > of the test suite. > > - testing for prereqs is now done using the "hash" built-in instead of > "which", > as suggested in [3]. > > Tested with and without dtach. A previous version of this series was also > without emacs/gpg in a chroot, but not this one :) > > [1] id:"874ny4fcdz@praet.org" > [2] id:"1317660447-27520-1-git-send-email-schno...@schnouki.net" > [3] id:"87zkgemodd@praet.org" > > Pieter Praet (4): > test: add 'GnuPG' prereq to dependent 'crypto' tests > test: add 'Emacs' prereq to dependent 'crypto' tests > test: add 'Emacs' prereq to dependent 'emacs' tests > test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer' > tests > > Thomas Jost (2): > test: define a helper function for defining prereqs on executables > test: check if emacs and dtach are available in test_emacs() > > test/crypto| 46 +++--- > test/emacs | 82 +-- > test/emacs-large-search-buffer |9 +++- > test/test-lib.sh | 17 > 4 files changed, 99 insertions(+), 55 deletions(-) > > -- > 1.7.7.3 > > ___ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch diff --git a/test/emacs b/test/emacs index ea641af..79edf01 100755 --- a/test/emacs +++ b/test/emacs @@ -244,7 +244,8 @@ test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"") (notmuch-test-wait) (notmuch-search-reply-to-thread) (test-output)' -sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' EMACS OUTPUT +test_have_prereq EMACS \ +&& sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: /' OUTPUT cat"$GNUPGHOME"/import.log 2>&1 test_debug "cat $GNUPGHOME/import.log" if (gpg --quick-random --version >/dev/null 2>&1) ; then @@ -41,7 +41,7 @@ add_gnupg_home () add_gnupg_home # get key fingerprint -FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10) +test_have_prereq GPG && FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10) # for some reason this is needed for emacs_deliver_message to work, # although I can't figure out why @@ -80,7 +80,7 @@ expected='[[[{"id": "X", {"id": 3, "content-type": "application/pg
Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
On Wed, 16 Nov 2011 10:53:42 -0800, Jameson Graef Rollins wrote: > On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost 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 pgpkXPnYrMIC2.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 0/6] Rebase of Pieter's "set test prereqs"
On Wed, 16 Nov 2011 15:33:49 +0100, Thomas Jost 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. pgpJ7dHmHkBJm.pgp Description: PGP signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
