From: Pat Notz Date: Wed, 22 Sep 2010 18:16:24 GMT Subject: Re: [PATCHv2 4/4] t7500: add tests of commit --squash Message-ID: In-Reply-To: On Wed, Sep 22, 2010 at 12:12 PM, Ævar Arnfjörð Bjarmason wrote: > On Wed, Sep 22, 2010 at 17:59, Pat Notz wrote: >> On Tue, Sep 21, 2010 at 2:36 PM, Ævar Arnfjörð Bjarmason >> wrote: >>> >>> On Tue, Sep 21, 2010 at 20:25, Pat Notz wrote: >>> >>> > +cat >editor <<\EOF >>> > +#!/bin/sh >>> > +sed -e "s/intermediate/edited/g" <"$1" >"$1-" >>> > +mv "$1-" "$1" >>> > +EOF >>> > +chmod 755 editor >>> > + >>> > +test_expect_success 'commit --squash works with -c' ' >>> > +       commit_for_rebase_autosquash_setup && >>> > +       EDITOR=./editor git commit --squash HEAD~1 -c HEAD && >>> > +       commit_msg_is "squash! target message subject lineedited commit" >>> > +' >>> >>> Why not put the editor in t/t7500/ and use test_set_editor() like the >>> other tests? >> >> The real reason is that I'm new enough that I wasn't aware of this >> pattern.  I saw what was done in t7501-commit.sh and followed along. >> I missed the use of test_set_editor() right there in t7500-commit.sh. >> Doh! >> >> I can certainly do that if it's preferred.  I must say, though, that I >> find it odd to put test inputs in a separate file in a separate >> directory from where the test transforms those into expected outputs. >> To see what the test is doing you have to load both files and trace >> through it. >> >> Still, I'd be happy to change do this if that's the preferred way. > > It's a bit odd, but it's best to following existing style within a > test. Then maybe submit fixup patches to fix the whole thing later. > Yeah, there's certainly value in doing that. I'll follow-up with a v3 and include the documentation changes I noticed in the other patches.