On Wed, Sep 19, 2012 at 01:56:55PM -0400, Jeff King wrote:
> On Wed, Sep 19, 2012 at 06:15:13PM +0100, Adam Spiers wrote:
> > This will allow us to test the test framework more thoroughly
> > without disrupting the top-level test metrics.
> I see this is prep for the next patch, and the parts pulling out the
> test-runs into functions make sense. But this hunk confuses me:
> > @@ -166,7 +176,7 @@ test_expect_success 'tests clean up even on failures' "
> >     test_must_fail ./failing-cleanup.sh >out 2>err &&
> >     ! test -s err &&
> >     ! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
> > -   sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
> > +   sed -e 's/Z$//' -e 's/^> //' >expect <<-EOF &&
> >     > not ok 1 - tests clean up even after a failure
> >     > #     Z
> >     > #     touch clean-after-failure &&
> 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

> I think our usual style is not to interpolate, and
> to do so only when we explicitly want it, which can prevent accidental
> errors due to missing quoting.

Right, that makes sense.  I'd vote to put it back in then.

> 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 ;-)  I've also noticed I can use test_must_fail
instead of introducing run_sub_test_lib_test_expecting_failures.

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.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to