From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Thomas Jost <schnouki@schnouki.net>, notmuch@notmuchmail.org
Subject: Re: [PATCH 0/9] test: (hopefully) better test prerequisites
In-Reply-To: <87d3craxnx.fsf@thor.loria.fr>
References: <1321494986-18998-1-git-send-email-dmitry.kurochkin@gmail.com>
	<87d3craxnx.fsf@thor.loria.fr>
User-Agent: Notmuch/0.10_rc1+9~g8270095 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 17 Nov 2011 15:45:55 +0400
Message-ID: <87ehx7nf9o.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Thu, 17 Nov 2011 10:46:58 +0100, Thomas Jost <schnouki@schnouki.net> wrote:
> On Thu, 17 Nov 2011 05:56:17 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Hi all.
> > 
> > The following patch series is an attempt to introduce proper
> > dependencies for external binaries in a less intrusive way than
> > [1].  The primary aim was to avoid changing every subtest that
> > uses external binaries.
> > 
> > There are still failing tests if a dependency is
> > missing (e.g. "Verify that sent messages are
> > saved/searchable (via FCC)" fails if there is no emacs).  It
> > happens because such tests depend on others which are skipped.
> > This issues are not addressed by this patch series.
> > 
> > If others do like the approach and it is pushed, I will work on
> > updating tests that use the old style prerequisites (atomicity).
> > 
> > A careful review is needed!
> > 
> > Regards,
> >   Dmitry
> > 
> > [1] id:"1321454035-22023-1-git-send-email-schnouki@schnouki.net"
> 
> Hi Dmitry,
> 
> This series looks quite good to me. It's a good approach, cleaner than
> explicitely adding the prereqs to each test as in my previous patches
> (and Pieter's).
> 
> Now, a few questions:
> 
> - same as Jamie: emacs_deliver_message hangs if dtach is not installed.
>   In my patches I had to do this: "test_have_prereq EMACS &&
>   emacs_deliver_message ...". Any idea how to handle this?
> 

Answered to Jamie already.  That is a separate bug which is fixed in [1].

> - what about indirect, "hidden" dependencies? The crypto test need to
>   have a signed message delivered by emacs, so actually *all* the crypto
>   tests depend on emacs. Right now, when dtach is not installed, the
>   first test ("emacs delivery of signed message") is skipped and all the
>   others fail. Would it be possible to handle this case without having
>   to add explicit prereqs?
> 

Dependencies between the tests are not trivial (e.g. a test expects
notmuch to have a message delivered by the previous one).  I see two
solutions here:

* Remove such dependencies.  E.g. the code which sends a message may be
  put in a function.  Every test who needs it will call that function.
  No dependency on a previous test.

* Declare explicit prerequisites in some tests and use them in other
  tests.  IMO this is the only proper way to handle dependencies between
  tests.

We can (and probably should) use both approaches to resolving such
dependencies.

We can make it easier to implement the latter approach: add a function
like test_other_subtest_prereq which takes ID of subtests this one
depends on.  This way every subtest implicitly provides a prerequisite
later test can depend on.  Still we would have to explicitly check the
prerequisite in every test case that needs it.  I see no way to get rid
of it.

Note that inter-subtest dependencies is an existing issue.  Currently,
you can skip a single test.  Then all others that depend on it would
fail.

Also, we can move common stuff that is used by all subtests to the test
initialization (before the first subtest).  Then if it fails, all
subtests would be skipped.  Because of this all crypto tests would be
skipped if gpg is not installed.

> - right now functions like test_expect_success can be used as
>   "test_expect_success COMMAND" or "test_expect_success PREREQ COMMAND".
>   If we use your approach (and I hope we will!), do we need to keep that
>   second syntax available in test-lib.sh too, or should we do some
>   cleanup to get rid of it?
> 

At first, I wanted to clean it up.  But then I decided to leave it.  The
"old-style" prerequisites are more general.  So this patch series does
not fully replace it.  Besides there are tests that use it now
(atomicity at least), I have not updated them yet.

We may use general prerequisites for inter-subtest dependencies (though,
we probably can make it a little more convenient, see above).

I am not sure if we should remove general prerequisites even if they are
not used.  They work and they do not hurt.  I will left it for others to
decide.

Regards,
  Dmitry

[1] id:"yf639dnsqtc.fsf@taco2.nixu.fi"

> Thanks,
> 
> -- 
> Thomas/Schnouki