From: Jeff King <peff@peff.net>
Date: Wed, 19 Sep 2012 19:37:08 GMT
Subject: Re: [PATCH v2 4/6] Refactor mechanics of testing in a sub test-lib
Message-ID: <20120919193708.GA21950@sigill.intra.peff.net>
In-Reply-To: <20120919184406.GC19246@atlantic.linksys.moosehall>

On Wed, Sep 19, 2012 at 07:44:06PM +0100, Adam Spiers wrote:

> > Is it just that you are dropping the '\' in all of the here-docs because
> > they are not needed?
> 
> Hmm, I think I previously misunderstood the point of the \\ due to
> never seeing that syntax before (since my Perl background taught me to
> write <<'EOF' instead).  I noticed that the tests all passed without
> it, and mistakenly assumed it had become unnecessary due to the
> refactoring.

OK. You can write 'EOF' in the shell, too, but we tend not to in this
project (and you can write \EOF in perl, but I agree that it is much
less common in perl code I have seen).

Looking at it again, it is actually quite subtle what is going on. We
wrap the outer test_expect_* calls in double-quotes so that the inner
ones can use single-quotes easily. But that means that technically the
contents of the here-doc _are_ interpolated. But not at test run-time,
but rather at the call to test_expect_*. And that is why we nee to use
"\\" instead of "\". So I think anybody trying to tweak these tests
using shell metacharacters is in for a surprise either way. I'm not sure
it is worth worrying about, though, as handling it would probably make
the existing tests less readable.

> > Also, why is this one not converted into a check_sub... invocation?
> 
> Because it was much further down in that file so I didn't notice it
> during the refactoring ;-)

OK. :)

> I've also noticed I can use test_must_fail instead of introducing
> run_sub_test_lib_test_expecting_failures.

Good catch. I didn't notice that, but it definitely makes sense to reuse
it.

> 
> So I'll have to re-roll 4--6 again.  Presumably I can just reply to
> [PATCH v2 4/6] with modified v3 versions without having to resend
> the first three in the series, which haven't changed.

It all looks sane to me. Thanks again.

-Peff