From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Jameson Graef Rollins <jrollins@finestructure.net>,
	Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH] test: make test_expect_equal_file() arguments flexible
In-Reply-To: <8739aum87u.fsf@gmail.com>
References: <1328080794-24670-1-git-send-email-dmitry.kurochkin@gmail.com>
	<87r4yfszx9.fsf@servo.finestructure.net>
	<87pqdync64.fsf@gmail.com> <m262fqc0wv.fsf@guru.guru-group.fi>
	<87k446n8ji.fsf@gmail.com> <87ehuetqjz.fsf@servo.finestructure.net>
	<8739aum87u.fsf@gmail.com>
User-Agent: Notmuch/0.11+139~gd9b7cab (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Thu, 02 Feb 2012 04:07:01 +0400
Message-ID: <87wr86ksii.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Thu, 02 Feb 2012 03:42:29 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi Jameson.
> 
> On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > > > 
> > > > There are at least these options here
> > > > 
> > > > 1) go through all ~100 places where test_expect_equal_file is used
> > > >    and fix the call order: quick look tells that the offending uses
> > > >    are in dump-restore, hooks, search-limiting and symbol-hiding.
> > > > 
> > > > 2) enforce "expected" filename has some format *and* fix all current
> > > >    uses of it. Add testbed_error () function which yells loudly ane exits...
> > > > 
> > > > 3) guess which is output and which is expected from args so that 
> > > >    machine helps tester here (for both diff output & copied files)a
> > > > 
> > > > 4) just copy compared files to some directory, those are named as
> > > >    basename of the original -- diff order still inconsistent.
> > > > 
> > > > 
> > > > I'd just go with option 1 and fix new *violations* when stumble upon one.
> > > > 
> > > 
> > > Option 1 does not solve the problem.  New violations would apper and
> > > need to be fixed again.  I am for option 2.
> > 
> > How is enforcing use of a particular filename better and more robust
> > than enforcing argument order?
> 
> Filename check is a way to make sure the argument order is correct.
> 
> >  You'll still have to force an arbitrary
> > heuristic.  And you'll still be vulnerable to people messing up the file
> > names (which actually seems easier to get wrong than messing up the
> > order).
> 
> Do you mean that people would start writing tests with filenames like:
> 
>   test_expect_equal_file EXPECTED1 EXPECTED2
> 
> ?  That is possible, of course.  But do you seriously believe that
> deliberately changing file names in a way that violates common sense and
> is inconsistent with all other code is "easier" than writing "EXPECTED
> OUTPUT" in the wrong order?  I do not think so.  And it would definately
> be easier to catch during review.
> 
> >  And you'll have to have more code to parse the argument
> > strings.
> 
> That is one case statement, with one non-empty case, with one error
> call.
> 
> >  And you'll still get inconsistent diffs.
> > 
> 
> No, you do not.  That is the point.
> 
> > If this is really a problem, I vote for 1.  In general, I am not in
> > favor of making the test suite more complicated than it needs to be.
> > 
> 
> Ok.  I plan to send a patch soon.  If my arguments do not convince
> enough people, I will move along.
> 

The new patches, based on idea suggested by Tomi [1], are here:

  id:"1328141050-30356-1-git-send-email-dmitry.kurochkin@gmail.com"

Regards,
  Dmitry

[1] id:"m28vknaq5l.fsf@guru.guru-group.fi"

> Regards,
>   Dmitry
> 
> > jamie.