From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Date: Tue, 28 Sep 2010 15:09:10 GMT
Subject: Re: [PATCH v3 1/3] tests: Prepare --textconv tests for correctly-failing conversion program
Message-ID: <20100928150910.GA30544@landau.phys.spbu.ru>
In-Reply-To: <20100928143928.GA8918@sigill.intra.peff.net>

On Tue, Sep 28, 2010 at 10:39:28AM -0400, Jeff King wrote:
> On Tue, Sep 28, 2010 at 06:35:40PM +0400, Kirill Smelkov wrote:
> 
> > > >  t/t4042-diff-textconv-caching.sh |    4 ++--
> > > 
> > > Why are we touching t4042 at all in this series? We are not actually
> > > adding any tests to it, AFAICT.
> > 
> > Because we want to catch potential wrong textconv invocation on non
> > "bin: " files there too?
> 
> But we don't actually add any tests that display the problem there, do
> we? And even if we wanted to test the diff implementation, wouldn't
> t4030 be the write place to do that? t4042 is specifically about
> textconv caching.

Yes, I hadn't added new tests there, but at least I've changed
already-in-there helper to bail out if it is called on non "binary"
files, so a small step, but still step forward, no?

And I've changed that helper after doing `git grep` for textconv, and
ideally we shouldn't keep those copy-pasted helpers in several tests (we
have 3 at present - in t4042, t8006 and t8007), but move them into one
common place some day.

So in order to at least keep copies consistent between each other, I've
changed them all in uniform manner.

That was my rationale.


Thanks,
Kirill